Rename tests.shaders.cross fixture to tests.shaderCompilation.comprehensiveGLSL#1724
Open
bghgary wants to merge 1 commit into
Open
Rename tests.shaders.cross fixture to tests.shaderCompilation.comprehensiveGLSL#1724bghgary wants to merge 1 commit into
bghgary wants to merge 1 commit into
Conversation
…comprehensiveGLSL
The `tests.shaders.cross` name doesn't follow the convention of every
other JS fixture in this directory, which mirrors the matching cpp file's
suite name (`tests.shaderCache.basicScene.ts` ↔ `Tests.ShaderCache.cpp`,
`tests.externalTexture.msaa.ts` ↔ `Tests.ExternalTexture.Msaa.cpp`).
The fixture is the comprehensive GLSL shader loaded by
`TEST(ShaderCompilation, CompileComprehensiveGLSL)` in
`Tests.ShaderCompilation.cpp`.
Rename to `tests.shaderCompilation.comprehensiveGLSL.{ts,js}` so the
fixture name matches the suite + asset descriptor convention.
CMakeLists, webpack entry, and the ScriptLoader path are updated to
match. No test-code change.
[Created by Copilot on behalf of @bghgary]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR renames the comprehensive GLSL shader-compilation JavaScript fixture so that the fixture naming in Apps/UnitTests/JavaScript/src/ mirrors the corresponding C++ test suite naming (specifically TEST(ShaderCompilation, CompileComprehensiveGLSL)).
Changes:
- Renamed the shader compilation fixture output/entry to
tests.shaderCompilation.comprehensiveGLSL. - Updated the C++ unit test to load the renamed fixture asset.
- Updated build plumbing (CMake asset list + webpack entry) and the committed
dist/bundle to match the new name.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Apps/UnitTests/Source/Tests.ShaderCompilation.cpp | Updates ScriptLoader::LoadScript path to the renamed JS asset. |
| Apps/UnitTests/JavaScript/webpack.config.js | Renames the webpack entry key and input TS file path. |
| Apps/UnitTests/JavaScript/src/tests.shaderCompilation.comprehensiveGLSL.ts | Adds the renamed TypeScript fixture source for comprehensive GLSL compilation. |
| Apps/UnitTests/JavaScript/dist/tests.shaderCompilation.comprehensiveGLSL.js | Updates the committed webpack output bundle to match the renamed entry/source. |
| Apps/UnitTests/CMakeLists.txt | Updates TEST_ASSETS list to copy the renamed dist/ JS file into the test assets. |
bghgary
changed the title
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.
Context
Every JS fixture in
Apps/UnitTests/JavaScript/src/mirrors the matching cpp file's suite name:tests.shaderCache.basicScene.ts↔Tests.ShaderCache.cpptests.externalTexture.msaa.ts↔Tests.ExternalTexture.Msaa.cpptests.shaders.cross.tsdoesn't follow this convention and reads as opaque. It's the comprehensive GLSL shader fixture loaded byTEST(ShaderCompilation, CompileComprehensiveGLSL)inTests.ShaderCompilation.cpp.Change
Rename
tests.shaders.cross.{ts,js}→tests.shaderCompilation.comprehensiveGLSL.{ts,js}. UpdatesCMakeLists.txtasset list,webpack.config.jsentry, and theScriptLoader::LoadScriptpath inTests.ShaderCompilation.cpp. No test-code change.[Created by Copilot on behalf of @bghgary]