diff --git a/CHANGELOG.md b/CHANGELOG.md index 36e47aa6..3cd2be11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ Instructions: Add a subsection under `[Unreleased]` for additions, fixes, change ## [Unreleased] +### Fixed + +- Failed individual asset generation (when core does not produce the expected output file) now raises an error and is no longer silently treated as a success. This prevents incorrectly caching incomplete asset generation results. + ### Changed - The default Asymptote generation method is now `local` (using the local `asy` binary) instead of `server`. This improves reliability since the server method has been subject to breakage whenever the Asymptote web interface is updated. Users who need the server method can still opt in via `asy-method="server"` in `project.ptx`. diff --git a/pretext/project/__init__.py b/pretext/project/__init__.py index c7c17ba4..e6e93deb 100644 --- a/pretext/project/__init__.py +++ b/pretext/project/__init__.py @@ -1292,10 +1292,20 @@ def generate_assets( log.debug(e, exc_info=True) # After all assets are generated, update the asset cache (but we shouldn't do this if we didn't generate any assets successfully) log.debug(f"Updated these assets successfully: {successful_assets}") - if len(successful_assets) > 0 and not xmlid: + unsuccessful_assets = [ + asset for asset in assets_to_generate if asset not in successful_assets + ] + log.debug(f"These assets were unsuccessful: {unsuccessful_assets}") + if not xmlid: # If the build limited by xmlid, then we don't know that all assets of that type were build correctly, so we only do this if not generating a subset. + # First create a list of unsuccessful assets to log: for asset_type in successful_assets: saved_asset_table[asset_type] = source_asset_table[asset_type] + for asset_type in unsuccessful_assets: + log.debug( + f"Asset type {asset_type} was requested to be generated, but was not generated successfully. This asset type will be regenerated on the next build, even if the source files have not changed, since we can't be sure that the existing generated assets are up to date." + ) + saved_asset_table.pop(asset_type, None) # Save the asset table to disk: self.save_asset_table(saved_asset_table) log.info("Finished generating assets.\n") diff --git a/pretext/project/generate.py b/pretext/project/generate.py index b6b6a3c6..f5bab71c 100644 --- a/pretext/project/generate.py +++ b/pretext/project/generate.py @@ -74,6 +74,10 @@ def individual_prefigure( f"Created prefigure diagram {annotations_output_file}; saving a copy to cache as {annotations_cache_file}" ) shutil.copy2(annotations_output_file, annotations_cache_file) + else: + msg = f"Failed to generate prefigure diagram {Path(pfdiagram).name} in {format} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_prefigure function") @@ -109,6 +113,10 @@ def individual_asymptote( f"Created asymptote diagram {output_file}; saving a copy to cache as {cache_file}" ) shutil.copy2(output_file, cache_file) + else: + msg = f"Failed to generate asymptote diagram {Path(asydiagram).name} in {outformat} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_asymptote function") @@ -146,6 +154,10 @@ def individual_sage( f"Created sageplot diagram {output_file}; saving a copy to cache as {cache_file}" ) shutil.copy2(output_file, cache_file) + else: + msg = f"Failed to generate sageplot diagram {Path(sageplot).name} in {outformat} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_sage function") @@ -191,6 +203,10 @@ def individual_latex_image( f"Created latex-image {output_files[ext]}; saving a copy to cache as {cache_files[ext]}" ) shutil.copy2(output_files[ext], cache_files[ext]) + else: + msg = f"Failed to generate latex-image {Path(latex_image).name} in {ext} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_latex function") diff --git a/tests/test_generate.py b/tests/test_generate.py new file mode 100644 index 00000000..7550ee72 --- /dev/null +++ b/tests/test_generate.py @@ -0,0 +1,225 @@ +""" +Tests to ensure that failed image generation is not reported as success. + +When a core conversion function fails to produce an output file, the +individual_* functions in pretext/project/generate.py should raise an +exception so that the asset type is not added to the generated asset cache. +""" + +from pathlib import Path +from unittest.mock import patch + +import pytest + +from pretext.project import generate + + +def _make_asset_file(tmp_path: Path, name: str = "test_asset.tex") -> Path: + """Create a minimal asset file in tmp_path for use in tests.""" + asset_file = tmp_path / name + asset_file.write_text("some asset content") + return asset_file + + +def test_individual_latex_image_raises_on_missing_output(tmp_path: Path) -> None: + """individual_latex_image should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "image.tex") + cache_dir = tmp_path / "cache" / "latex-image" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + # Patch core so it does NOT create the output file. + with patch("pretext.project.generate.core.individual_latex_image_conversion"): + with pytest.raises(Exception, match="Failed to generate latex-image"): + generate.individual_latex_image( + latex_image=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + method="xelatex", + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_latex_image_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_latex_image should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "image.tex") + cache_dir = tmp_path / "cache" / "latex-image" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + def fake_conversion( + latex_image: str, outformat: str, dest_dir: Path, method: str + ) -> None: + # Simulate a successful conversion by creating the expected output file. + (dest_dir / Path(latex_image).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_latex_image_conversion", + side_effect=fake_conversion, + ): + # Should not raise. + generate.individual_latex_image( + latex_image=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + method="xelatex", + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_asymptote_raises_on_missing_output(tmp_path: Path) -> None: + """individual_asymptote should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "diagram.asy") + cache_dir = tmp_path / "cache" / "asymptote" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + with patch("pretext.project.generate.core.individual_asymptote_conversion"): + with pytest.raises(Exception, match="Failed to generate asymptote diagram"): + generate.individual_asymptote( + asydiagram=str(asset_file), + outformat="svg", + method="server", + asy_cli=[], + asyversion="", + alberta="", + dest_dir=dest_dir, + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_asymptote_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_asymptote should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "diagram.asy") + cache_dir = tmp_path / "cache" / "asymptote" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + def fake_conversion( + asydiagram: str, + outformat: str, + method: str, + asy_cli: list, + asyversion: str, + alberta: str, + dest_dir: Path, + ) -> None: + (dest_dir / Path(asydiagram).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_asymptote_conversion", + side_effect=fake_conversion, + ): + generate.individual_asymptote( + asydiagram=str(asset_file), + outformat="svg", + method="server", + asy_cli=[], + asyversion="", + alberta="", + dest_dir=dest_dir, + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_sage_raises_on_missing_output(tmp_path: Path) -> None: + """individual_sage should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "plot.sage") + cache_dir = tmp_path / "cache" / "sageplot" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + with patch("pretext.project.generate.core.individual_sage_conversion"): + with pytest.raises(Exception, match="Failed to generate sageplot diagram"): + generate.individual_sage( + sageplot=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + sage_executable_cmd=["sage"], + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_sage_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_sage should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "plot.sage") + cache_dir = tmp_path / "cache" / "sageplot" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + def fake_conversion( + sageplot: str, + outformat: str, + dest_dir: Path, + sage_executable_cmd: list, + ) -> None: + (dest_dir / Path(sageplot).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_sage_conversion", + side_effect=fake_conversion, + ): + generate.individual_sage( + sageplot=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + sage_executable_cmd=["sage"], + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_prefigure_raises_on_missing_output(tmp_path: Path) -> None: + """individual_prefigure should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "figure.xml") + cache_dir = tmp_path / "cache" / "prefigure" + cache_dir.mkdir(parents=True) + tmp_dir = tmp_path / "tmp_work" + tmp_dir.mkdir() + + with patch("pretext.project.generate.core.individual_prefigure_conversion"): + with pytest.raises(Exception, match="Failed to generate prefigure diagram"): + generate.individual_prefigure( + pfdiagram=str(asset_file), + outformat="svg", + tmp_dir=str(tmp_dir), + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_prefigure_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_prefigure should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "figure.xml") + cache_dir = tmp_path / "cache" / "prefigure" + cache_dir.mkdir(parents=True) + tmp_dir = tmp_path / "tmp_work" + tmp_dir.mkdir() + + def fake_conversion(pfdiagram: str, outformat: str) -> None: + output_dir = tmp_dir / "output" + output_dir.mkdir(parents=True, exist_ok=True) + (output_dir / Path(pfdiagram).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_prefigure_conversion", + side_effect=fake_conversion, + ): + generate.individual_prefigure( + pfdiagram=str(asset_file), + outformat="svg", + tmp_dir=str(tmp_dir), + cache_dir=tmp_path / "cache", + skip_cache=True, + )