Repository navigation
test(gapic-generator): manage Showcase test alignment when PyPI google-api-core lags monorepo - #18864
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to detect whether the installed google-api-core package matches the monorepo source, skipping specific system and span compliance tests when a mismatch is found. The review feedback suggests replacing binary file comparison (filecmp.cmp) with text-based comparison (read_text) to prevent false mismatches caused by cross-platform line ending differences (CRLF vs LF) on Windows.
00f8d5b to
265c16d
Compare
91d5cce to
e48ff4a
Compare
| pytest.skip( | ||
| "PyPI google-api-core differs from monorepo; " | ||
| "strict span attribute matrix is validated in showcase-canary" | ||
| ) |
There was a problem hiding this comment.
It seems like these tests will always be skipped on non-canary, even years from now, when this feature has long-since been released. Do you think we should add a parameter to this decorator with a future release version, so the tests can enable themselves on that version or greater? @pytest.mark.requires_core_version("2.55.0")
That would give us a signal when the marker can be removed, and allow the tests to run in the future if forgotten
I'm still getting my head around this though. Let me know if that would cause issues, or if I'm misunderstanding
There was a problem hiding this comment.
I just pushed an update to ensure that assert_span_matches_row always runs first, and we only skip a test in showcase if it raises AssertionError AND the installed PyPI google-api-core code differs from the local monorepo code:
How does this play out across various scenarios?:
Unrelated google-api-core changes: If someone the code in a way that is completely unrelated to any of the observability tests that happen in test_span_compliance.py such as changing a docstring, then assert_span_matches_row still runs and we expected to pass for all 38 rows. Thus 0 tests are skipped.
Unreleased google-api-core changes that DO affect spans: All unaffected rows in the matrix still run and pass on showcase; only matrix rows whose assertions fail against the older PyPI package are skipped in showcase (NOTE: showcase-canary runs against local google-api-core and still fully enforces all 38 rows).
After google-api-core changes are released to PyPI: The PyPI and monorepo files match again (INSTALLED_CORE_MATCHES_MONOREPO == True), so all 38 rows enforce strict assertions on both showcase and showcase-canary automatically — without needing to track or clean up version numbers over time.
There was a problem hiding this comment.
So it sounds like failing tests would be marked as skipped? That could hide real errors in the future, right? That sounds dangerous
My suggestion is to add a quick min_version parameter to the decorator, and actually raise errors after that version is released, rather than skipping
If that's too difficult, I think the old behavior of always skipping is actually better, because skips are more likely to be noticed and result in the decorator being removed, rather than silent guaranteed passes
…e-api-core lags monorepo
…ns before skipping
…lure when core differs
…mismatch context manager
dbb6d01 to
1f69e89
Compare
daniel-sanche
left a comment
There was a problem hiding this comment.
Approving because this is test code, and I don't want to block the PRs waiting on this. But consider my comments about reverting the "skip on fail" behavior
|
I will revisit with you and we can find a better long-term solution, thanks Daniel. |
Problem
In the monorepo CI pipeline,
gapic-generatorruns two Showcase system test suites:showcase: Installs the latest released version ofgoogle-api-corefrom PyPI to verify backward compatibility of generated clients.showcase-canary: Installs the local monorepopackages/google-api-core(INSTALL_LOCAL_CORE=true) to verify integration between currentgapic-generatorandgoogle-api-corechanges.Because
google-api-coreandgapic-generatorrelease on independent schedules,packages/google-api-corein the monorepo frequently contains unreleased changes that are not yet available on PyPI. When a Showcase system test asserts behavior that depends on unreleasedgoogle-api-corechanges,showcase-canarypasses against the local monorepo source, but the non-canaryshowcasejob fails against the older PyPI release until the nextgoogle-api-corerelease is published.Previously, individual test files worked around this misalignment using ad-hoc, feature-specific
hasattrchecks.Solution
This pull request introduces a general, feature-agnostic mechanism in
packages/gapic-generator/tests/system/conftest.pyto detect and manage misalignment between the installed PyPIgoogle-api-corepackage and the monorepopackages/google-api-coresource:Monorepo vs. Installed
google-api-coreSync Check (conftest.py):_installed_core_matches_monorepo()andINSTALLED_CORE_MATCHES_MONOREPOto compare the activegoogle.api_coreinstallation againstpackages/google-api-core/google/api_corein the monorepo.skip_on_core_mismatch()context manager that always runs the wrapped assertion(s) first and only converts anAssertionErrorintopytest.skip(...)when the installed PyPIgoogle-api-corelags behind the monorepo source:google-api-coredoes not affect a test assertion, the assertion runs and passes without skipping.google-api-corehas unreleased changes, only that failing assertion is skipped in non-canaryshowcase, whileshowcase-canary(INSTALL_LOCAL_CORE=true) strictly enforces it.google-api-corerelease is published to PyPI,INSTALLED_CORE_MATCHES_MONOREPOautomatically becomesTrueagain and all assertions are strictly enforced in bothshowcaseandshowcase-canarywith no version-marker cleanup required.Initial Application (
test_span_compliance.py):assert_span_matches_row) inwith conftest.skip_on_core_mismatch():so upcoming pull requests that adjustgoogle-api-coretelemetry attributes do not fail non-canaryshowcaseCI prior to the nextgoogle-api-corePyPI release, while still executing all RPC scenarios and unaffected matrix rows against PyPIgoogle-api-core.