Metashade render test integration#12
Merged
Merged
Conversation
…ce conflicts for VS Code test discovery
…py, removing local duplicate definitions
…y clashes, define helper functions locally, and keep tests DRY
…TEST_REF_DIR in workspace .env
ppenenko
changed the title
ppenenko
changed the title
Member
Author
|
Relying on metashade/metashade#197 |
There was a problem hiding this comment.
Pull request overview
Refactors the contrib render test setup to avoid pytest namespace clashes during VS Code discovery, share render/baseline logic between standard and Metashade-driven tests, and redirect Metashade reference artifacts into the parent repo.
Changes:
- Moved render test collection helpers and element skip logic into
contrib/tests/test_render.pyand introduced sharedrun_render_test_file(...). - Added a new Metashade override render test (
test_render_metashade.py) plus checked-in Metashade reference/override artifacts undercontrib/tests/metashade_ref/. - Updated workspace configuration (
.env, VS Code settings/launch) to wire Metashade reference redirection and includecontrib/testsin test discovery.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| contrib/tests/test_render.py | Inlines helper functions previously imported from conftest and adds run_render_test_file(...) + stream helpers. |
| contrib/tests/test_render_metashade.py | New targeted Metashade override render tests for Schlick materials. |
| contrib/tests/metashade_ref/test_metashade_add_color3_genglsl_impl.mtlx | Adds Metashade-generated node implementation descriptor. |
| contrib/tests/metashade_ref/test_metashade_add_color3_genglsl_impl.glsl | Adds Metashade-generated GLSL implementation. |
| contrib/tests/metashade_ref/test_metashade_add_color3_defs.mtlx | Adds Metashade-generated node definition. |
| contrib/tests/metashade_ref/test_dummy_purple_genglsl_impl.mtlx | Adds Metashade-generated dummy implementation descriptor. |
| contrib/tests/metashade_ref/test_dummy_purple_genglsl_impl.glsl | Adds Metashade-generated dummy GLSL implementation. |
| contrib/tests/metashade_ref/test_dummy_purple_defs.mtlx | Adds Metashade-generated dummy node definition. |
| contrib/tests/metashade_ref/source_code_nodes.txt | Adds a reference list of source-code nodes used by Metashade tests. |
| contrib/tests/metashade_ref/mx_generalized_schlick_bsdf_metashade_genglsl_impl.mtlx | Adds Metashade wrapper implementation descriptor for Schlick BSDF override. |
| contrib/tests/metashade_ref/mx_generalized_schlick_bsdf_metashade_genglsl_impl.glsl | Adds Metashade wrapper GLSL that includes/calls the stdlib Schlick implementation. |
| contrib/tests/conftest.py | Removes duplicated stream helper logic and imports add_additional_test_streams from test_render. |
| .vscode/settings.json | Points VS Code Python to .env and includes contrib/tests in pytest args. |
| .vscode/launch.json | Updates Graph Editor debug launch to use the new Metashade reference folder. |
| .env | Sets METASHADE_MTLX_PYTEST_REF_DIR for VS Code-driven runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+73
to
+83
| @pytest.fixture(scope="session") | ||
| def output_dir(self, request, repo_root) -> Path: | ||
| """Override output_dir to place Metashade results under their own root.""" | ||
| opt = request.config.getoption("--output-dir") | ||
| if opt: | ||
| path = Path(opt) / "metashade_schlick" | ||
| else: | ||
| path = repo_root / "contrib" / "renders" / "metashade_schlick" | ||
| path.mkdir(parents=True, exist_ok=True) | ||
| return path | ||
|
|
Comment on lines
+143
to
+152
| """Test all renderable elements in a stdlib material file using the Schlick override.""" | ||
| run_render_test_file( | ||
| mtlx_file=mtlx_file, | ||
| subtests=subtests, | ||
| renderer=schlick_renderer, | ||
| data_library=schlick_stdlib, | ||
| search_path=schlick_search_path, | ||
| output_dir=output_dir, | ||
| assert_image_matches_baseline=assert_image_matches_baseline | ||
| ) |
Comment on lines
+107
to
+113
| """ | ||
| Add additional test streams required by MaterialX test suite. | ||
|
|
||
| This mirrors the C++ addAdditionalTestStreams() in RenderUtil.cpp, | ||
| adding geometry attributes needed by geompropvalue, streams, and | ||
| struct_texcoord tests. | ||
| """ |
…d update add_additional_test_streams docstring
Comment on lines
+31
to
+37
| files = [] | ||
| for rel_path_str in targeted: | ||
| mtlx_file = materials_root / rel_path_str | ||
| if mtlx_file.exists(): | ||
| files.append(pytest.param(mtlx_file, id=rel_path_str)) | ||
|
|
||
| return files |
Comment on lines
+78
to
+82
| # 2. Add Metashade override path | ||
| metashade_mtlx_path = repo_root / "contrib" / "tests" / "metashade_ref" | ||
| if metashade_mtlx_path.exists(): | ||
| custom_sp.append(metashade_mtlx_path.as_posix()) | ||
|
|
Comment on lines
+90
to
+94
| # 1. Load Metashade Schlick override first | ||
| override_mtlx = repo_root / "contrib" / "tests" / "metashade_ref" / "mx_generalized_schlick_bsdf_metashade_genglsl_impl.mtlx" | ||
| if override_mtlx.exists(): | ||
| mx.readFromXmlFile(lib, override_mtlx.as_posix()) | ||
|
|
- Remove unused `List` import from test_render.py (comment #4) - get_schlick_test_files(): skip explicitly if no targeted files found (comment #5) - schlick_search_path fixture: skip if metashade_ref dir is missing (comment #6) - schlick_stdlib fixture: skip if Schlick override .mtlx file is missing (comment #7)
These files (metashade_ref/ dir and override .mtlx) are committed to the repo, not optional env dependencies. Their absence means the environment is broken, which should surface as a failure, not a silent skip.
Comment on lines
+176
to
177
| _add_stream_if_missing(mesh, f"i_{GEOMPROP}_geompropvalue_color4", GEOMPROP, 1, 4, fill_color4) | ||
| def find_renderable_elements(doc): |
Comment on lines
+128
to
+130
| # Second UV set - copy from texcoord0 | ||
| _add_stream_if_missing(mesh, f"i_{TEXCOORD}_1", TEXCOORD, 1, 2, | ||
| lambda d: [d.__setitem__(i, uv[i]) for i in range(len(uv))]) |
Member
Author
|
Follow-up issue for the next step: #13 — restructuring overrides into per-environment subdirectories with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the Metashade render tests integration in the
contribfolder. It resolves Pytest namespace clashes during VS Code test discovery, isolates test outputs, keeps the codebase DRY, and prevents the tests from dirtying the Metashade submodule when run.Key Enhancements
Resolved Pytest Namespace Clashes:
from conftest import ...imports from the test files. This prevents Python/Pytest from loadingcontrib/metashade/tests/conftest.pyas the globalconftestmodule cache key undersys.modules, which previously broke test discovery in VS Code.get_repo_root,get_stdlib_files,get_adsk_files,should_skip_element,get_element_skip_reason) are now defined locally intest_render.pywithout requiring external imports.Clean Submodule Environment (No Submodule Dirtying):
contrib/metashadesubmodule to commit779bc46fa68be45068447ecd315ac88664f420aa, which introduces theMETASHADE_MTLX_PYTEST_REF_DIRenvironment variable..envfile to setMETASHADE_MTLX_PYTEST_REF_DIR=contrib/tests/metashade_ref..vscode/settings.jsonto explicitly point to this.envfile via"python.envFile".contrib/tests/metashade_ref/inside the parent repository rather than modifying thecontrib/metashadesubmodule, keeping it completely clean.source_code_nodes.txt, test purple/add implementations) to the parent repository undercontrib/tests/metashade_ref/.DRY Shared Render Logic:
contrib/tests/test_render.pyandcontrib/tests/test_render_metashade.pyto share a common rendering and baseline image comparison helper functionrun_render_test_file(...).add_additional_test_streams) is defined intest_render.pyas a regular function and imported cleanly inconftest.py(locally inside the fixture) andtest_render_metashade.pywithout any complex fixture factories or circular imports.Isolated Render Outputs:
contrib/renders/metashade_schlick/) to avoid stomping on standard render test outputs.VS Code Test Explorer Crash Fix on Windows:
"-n", "auto"argument frompython.testing.pytestArgsin.vscode/settings.jsonon Windows. Concurrently launching 20+ worker processes for ultra-fast tests previously caused named pipe buffer overflows, crashing test runs in Test Explorer.Verification
All 227 standard and 162 Metashade unit tests execute and pass cleanly: