Skip to content

Conversation

@Sionigdha
Copy link
Contributor

This PR is a small follow-up to the introduction of pytest.mark.network.

It applies the marker to an additional test that clearly depends on external
network access, following the same conservative, case-by-case approach
discussed in #10886.



@pytest.mark.network
@pytest.mark.require_curl
Copy link
Member

Choose a reason for hiding this comment

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

require_curl is not needed here since pytestmark = pytest.mark.require_driver("WMS") at line 28 implies it (the WMS driver is only enabled if CURL is enabled)

That pull request would only be worth it if generalized to marking (temptively) all tests that perform an external network access (tests doing localhost networking shouldn't be tagged with it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the clarification.
You’re right , require_curl is redundant here given require_driver("WMS"). I’ll drop it.

Regarding scope,I agree that the marker is only really useful if applied more broadly. I kept this PR minimal as a follow-up example, but I’m happy to extend it to other tests that clearly perform external (non-localhost) network access if that’s preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All non-fuzzing CI checks are now green.

The remaining CIFuzz failure appears unrelated to the changes in this PR, which only affect pytest markers and do not touch any fuzzed code paths.

Happy to re-run or adjust if you think it’s related.

Copy link
Member

Choose a reason for hiding this comment

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

The remaining CIFuzz failure appears unrelated to the changes in this PR,

yes that is unrelated. you can ignore that and concentrate on adding more markers where relevant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All relevant tests performing external (non-localhost) network access have now been tagged, and CI is green.

I’ll leave it at this unless you’d like me to extend the marker coverage further.

Copy link
Member

Choose a reason for hiding this comment

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

There are tests in the following files that would also benefit from the marker:

  • autotest/gdrivers/gdalhttp.py
  • autotest/gcore/vsis3.py
  • autotest/gcore/vsigs.py
  • autotest/gcore/vsicurl.py
  • autotest/gcore/vsicurl_streaming.py
  • autotest/ogr/ogr_sqlite.py
  • autotest/ogr/ogr_shape.py
  • autotest/ogr/ogr_shape_sbn.py
  • autotest/ogr/ogr_parquet.py

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.

2 participants