inveniosoftware / pytest-invenio

Pytest fixtures for Invenio.
https://pytest-invenio.readthedocs.io
Other
3 stars 22 forks source link

fixtures: disable rate-limiting in config #95

Closed slint closed 1 year ago

slint commented 1 year ago

:heart: Thank you for your contribution!

Description

Please describe briefly your pull request.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Third-party code

If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.
zzacharo commented 1 year ago

The UI app already has it disabled.

This might break some places where the rate limiting is being tested. Search

@slint can you verify that tests will break and coordinate the release of this package if so? i.e fix the tests once the package is released or create a separate fixture with disabled rate limiting to avoid the impact?

slint commented 1 year ago

I checked locally and all tests on invenio-app are passing with master of pytest-invenio (i.e. including the changes of this PR). invenio-app is actually pretty "special" and the tests are not using pytest-invenio that much. The usual practice is to actually use them in combination in modules to get a Flask application factory quickly up.

The other cases in the search query are testing either explicitly 429 responses generated by the module itself in tests, or are just listings of possible error codes.

AFAIK, we can make a release and remove the extra config from https://github.com/inveniosoftware/invenio-rdm-records/pull/1223/commits/44c2b482c35e4b439982fcf15d89a89b6347eed1.

I'll coordinate the release :)