refactor(resolver): relocate httpresolver test double out of product surface - #133
Merged
Merged
Conversation
…surface The repository-local HTTP resolver was shipped under plugins/ despite being an integration-test double, which put it in the product surface, the release matrix, and its own CI cells. Move it to tests/plugins/httpresolver and drop it from the product build: remove the top-level add_subdirectory, its release metadata assertions, and its standalone CI cells. The COPC Tier 1 integration test now builds it transitively under USDGEO_BUILD_TESTS, keeping the required gate independent of external resolver repositories. Its openstrata.plugin.yaml is removed since it is no longer a published plugin. Add TryBuildPointCloudCacheLayout so producer and consumer tests derive resolver-backed cache entries from the same descriptor contract, and rework the resolver cache Tier 1 coverage to assert hits, incomplete and corrupted entry invalidation, and validation-token changes through cache artifacts rather than process-local counters, which are not shared across the FileFormat DLL boundary on Windows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The relocation commit documented Tier 1 resolver contract tests as the "required CI gate" and described the test double as built transitively by the COPC integration test in CI. Neither is true. Every cell in openstrata.ci.yaml is a per-plugin bundle build rooted at plugins/pointcloud-<x>/CMakeLists.txt, which is a standalone project() that never declares the USDGEO_BUILD_TESTS option. The option is undefined there, so the guarded block never runs and neither pointcloudCopc_tests nor the resolver fixture is compiled in CI. USDGEO_BUILD_TESTS defaults to ON only in the root CMakeLists.txt, which is what the local `ost build` gate configures. Restate Tier 1 as the required local gate across WORKSPACE.md, RESOLVER_SOURCE.md, infrastructure-maturity.md, and CHANGELOG.md, and record that wiring it into the CI matrix is outstanding follow-up work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
The repository-local HTTP resolver lived under
plugins/despite being an integration-test double. That placement put it in the product surface, the release metadata matrix, and its own standalone CI cells.This moves it to
tests/plugins/httpresolverand removes it from the product build:add_subdirectory(plugins/httpresolver).tools/check_release_metadata.py.openstrata.ci.yamlandost-source-ci.yml.openstrata.plugin.yaml— it is no longer a published plugin.openstrata_install_plugin_bundle, which fixes a real prior bug: the double was being installed into the shipped COPC product stage.USDGEO_BUILD_TESTSin the root build, so the local gate stays independent of external resolver repositories.Also included:
TryBuildPointCloudCacheLayoutso producer and consumer tests derive resolver-backed cache entries from the same descriptor contract.usd-http-resolverimplementation and its resolver-neutralArAssetInfoidentity contract.Known gap: Tier 1 is not in the CI matrix
Review of the first commit caught that it claimed Tier 1 was the "required CI gate" and that the fixture was built transitively by CI. Neither is true, and the second commit corrects the docs rather than overstating coverage.
Every cell in
openstrata.ci.yamlis a per-plugin bundle build rooted atplugins/pointcloud-<x>/CMakeLists.txt, which is a standaloneproject()that never declares theUSDGEO_BUILD_TESTSoption. Undefined there means the guarded block never runs, so neitherpointcloudCopc_testsnor the resolver fixture is compiled in CI. The option defaultsONonly in the rootCMakeLists.txt, which is what the localost configure && ost build && ost testgate configures.Two consequences worth naming explicitly:
pointcloudCopc_testswas always behind the guard, so Tier 1 has never run in CI.add_subdirectorymeant COPC cells at least compiledHttpResolver.cpp. Now nothing in CI does.Wiring Tier 1 into CI needs a new root-build cell with
ctest, which costs billed runner minutes and is left as follow-up.Test plan
python tools/check_release_metadata.py→ consistent at 0.9.0ost configure && ost build && ost testto exercise Tier 1plugins/httpresolverNote: I did not run a local C++ build; the CMake path changes are unverified by CI for the reason above.
Follow-ups
ctest).tests/test_pointcloud_copc.cppwritescache.manifestas the literalcommitted\n, while the real producer (WriteManifest,tools/usd-pointcloud-convert/main.cpp:442) writes a structured manifest with payload enumeration. It passes only becauseInspectMarkerchecks existence and file type, never content — so Tier 1 does not exercise the producer/consumer manifest contract.🤖 Generated with Claude Code