Skip to content

Fix the install_requires test that stopped matching urllib3 once it had a version bound - #3769

Open
brendancol wants to merge 1 commit into
mainfrom
fix-urllib3-test
Open

brendancol wants to merge 1 commit into
mainfrom
fix-urllib3-test

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

test_install_requires_lists_urllib3 asserts the literal string urllib3 is one of the install_requires lines. Since #3702 the line is urllib3 >=2.7, so the test fails on main whenever the integration directory is collected. It compares the first token now.

Found while running the full geotiff suite locally for #3768.

Test plan:

  • pytest xrspatial/geotiff/tests/integration/test_http_sources.py -k test_install_requires_lists_urllib3 passes locally

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Fix the install_requires test that stopped matching urllib3 once it had a version bound

One-line test change. Checked the surrounding code and the history.

Blockers (must fix before merge)

  • None.

Suggestions (should fix, not blocking)

  • None.

Nits (optional improvements)

  • xrspatial/geotiff/tests/integration/test_http_sources.py:2158: the test skips when setup.cfg is not next to the package, which is why CI (non-editable install) never saw the failure. If the guard is meant to be a real gate, the skip could become a failure on CI, but that is a separate decision.

What looks good

  • Comparing the first whitespace-separated token keeps the test's intent (urllib3 is declared) while allowing any version specifier, which is what #3702 introduced.
  • Passes locally against the current urllib3 >=2.7 line.

Checklist

  • Algorithm matches reference/paper (n/a)
  • All implemented backends produce consistent results (n/a)
  • NaN handling is correct (n/a)
  • Edge cases are covered by tests (n/a)
  • Dask chunk boundaries handled correctly (n/a)
  • No premature materialization or unnecessary copies (n/a)
  • Benchmark exists or is not needed
  • README feature matrix updated (n/a)
  • Docstrings present and accurate

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant