test(nwis): mock the remaining live NWIS tests (stop the CI flakes)#334
Merged
thodson-usgs merged 1 commit intoJun 24, 2026
Merged
Conversation
The legacy NWIS getters hit waterservices.usgs.gov, so several tests made live calls and flaked CI on transient outages (the recurring nwis/site connect timeouts). NWIS is deprecated (removal ~2027), so rather than maintain live coverage, convert these tests to offline fixtures: - nwis_test.py: mock TestMetaData (site_info → what_sites → /site), TestSiteseriesCatalogOutput (/site, with a captured seriesCatalogOutput fixture vs. the basic one), and TestTZ (/site + /iv). Remove the pure live-smoke test_nwis_service_live (nothing to assert once offline). Drop the now-unneeded module-level flaky_api marker. - utils_test.py: mock Test_query — a 414 → URLTooLong via the nwis.get_iv getter path (DOI-USGS#64), and the User-Agent header check. Drop flaky_api. - Add tests/data/nwis_site_seriescatalog.txt (a small captured seriesCatalogOutput=True RDB response). The module is now fully offline (45 + 36 tests, ~1s, no network), preserving the DOI-USGS#34 / DOI-USGS#60 / DOI-USGS#73 regression intent without the flakes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd
thodson-usgs
added a commit
to thodson-usgs/dataretrieval-python
that referenced
this pull request
Jun 24, 2026
Follow-up to DOI-USGS#334 (mocking the live NWIS tests). Once mocked, two NWIS classes became tautological — they assert the columns/shape of the fixture they feed, exercising no production logic the mocked-out parse doesn't already cover: - nwis_test.py: delete TestSiteseriesCatalogOutput (DOI-USGS#34 — RDB column parsing is covered by rdb_test.py + TestReadRdb, and the seriesCatalogOutput param is not exercised because the mock returns the fixture regardless) and TestTZ (DOI-USGS#60 — now identical to test_iv_service_answer: same fixture, same datetime-index assertion). Drop the orphaned nwis_site_seriescatalog.txt fixture and the now-unused imports. - waterdata_filters_test.py: delete test_construct_filter_passthrough — `filter` is forwarded verbatim, so it is subsumed by the parametrized test_construct_filter_on_all_ogc_services (8 services + filter-lang) and the long-filter fan-out tests. A 4-agent review of the full suite (17 files) found the rest well-curated; these are the only defensible deletions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd
thodson-usgs
added a commit
that referenced
this pull request
Jun 24, 2026
Follow-up to #334 (mocking the live NWIS tests). Once mocked, two NWIS classes became tautological — they assert the columns/shape of the fixture they feed, exercising no production logic the mocked-out parse doesn't already cover: - nwis_test.py: delete TestSiteseriesCatalogOutput (#34 — RDB column parsing is covered by rdb_test.py + TestReadRdb, and the seriesCatalogOutput param is not exercised because the mock returns the fixture regardless) and TestTZ (#60 — now identical to test_iv_service_answer: same fixture, same datetime-index assertion). Drop the orphaned nwis_site_seriescatalog.txt fixture and the now-unused imports. - waterdata_filters_test.py: delete test_construct_filter_passthrough — `filter` is forwarded verbatim, so it is subsumed by the parametrized test_construct_filter_on_all_ogc_services (8 services + filter-lang) and the long-filter fan-out tests. A 4-agent review of the full suite (17 files) found the rest well-curated; these are the only defensible deletions. Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd Co-authored-by: Claude Opus 4.8 <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
1 participant
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.
Why
nwisis the deprecated legacy module (removal scheduled ~2027). Its tests hitwaterservices.usgs.govdirectly, so transient upstream outages flaked CI evenwith the
flaky_apiretry marker. Per the guidance "deprecated, no extramaintenance, but don't break it accidentally," these are mocked (keeps the
breakage-detection coverage, kills the flakes) rather than deleted — except one
pure live-smoke test that has nothing to assert offline.
Changes
tests/nwis_test.py— waspytestmark = flaky_api(module-wide live):TestMetaData(6) —assert md.site_infolazily re-querieswhat_sites→/site; now mocked with the existingwaterservices_site.txtRDB fixture.(These are the 5 that failed CI on feat(waterdata): get_queryables + queryables monitor + passthrough enablement #333.)
TestSiteseriesCatalogOutput(5) —/siteviaget_record/get_info; mockedwith a new
seriesCatalogOutput=Truefixture (begin/end/count columns) for theTrue cases and the basic fixture for the default cases.
TestTZ(2) —/site+/iv; mocked (reusesnwis_iv_mock.json).test_nwis_service_live— a pure live round-trip smoke test;meaningless once mocked.
flaky_apimarker and the now-unused import.tests/utils_test.py—Test_querywas@flaky_api:test_url_too_long(Error parsing JSON #64) — mock a414→ asserts the typedURLTooLongviathe
nwis.get_ivgetter path.test_header— mock a200→ asserts theUser-Agentheader is sent.@flaky_api+ the unused import.tests/data/nwis_site_seriescatalog.txt— small captured (3-row)seriesCatalogOutput=TrueRDB response.Result
Both modules are now fully offline: 45 + 36 tests, ~1s, no network. ruff /
mypy clean. The
#34/#60/#73regression intent is preserved without theflakes. (NGWMN / Water Data live tests are out of scope — this PR is NWIS-only.)
Note
The
flaky_apimarker inconftest.pyis unchanged — it's still used by theremaining live-test modules (
ngwmn_test,waterdata_test).🤖 Generated with Claude Code