Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test_load_sso_credentials_without_cache #1041

Merged

Conversation

jakkdl
Copy link
Contributor

@jakkdl jakkdl commented Sep 19, 2023

Description of Change

This test appears to have been completely broken since it was written in 2020, but it wasn't marked with any fixtures so it got ignored in CI and you only notice it if running all tests or you look at codecov for the lines in question: https://app.codecov.io/gh/aio-libs/aiobotocore/blob/master/tests%2Fboto_tests%2Ftest_credentials.py#L1256
I simply copied the relevant code from the test below it and it started working, so I assume this is what it was meant to be.

Assumptions

That this test was intended to ... do something, anything.

Checklist for All Submissions

@jakkdl
Copy link
Contributor Author

jakkdl commented Sep 20, 2023

The failing test seems unrelated to code changes - we get a warning about path_to_write_report which was removed in 3.1.2, but that's not new and not the cause of the error.

But codecov uploading seemed to work in #1039, which was also opened from a fork (so it's not about secrets not being available in forks or something), so I'm not sure what might be different. Me being a first-time contributor to the repo?? Random fail??

@thehesiod
Copy link
Collaborator

thehesiod commented Sep 20, 2023

re-ran, looks like error is ImportError: cannot import name '_legacy_validators' from 'jsonschema'

@jakkdl
Copy link
Contributor Author

jakkdl commented Sep 22, 2023

re-ran, looks like error is ImportError: cannot import name '_legacy_validators' from 'jsonschema'

This seems to be jsonschema having renamed that in 4.19.1 (without mentioning in the changelog): python-jsonschema/jsonschema@v4.19.0...v4.19.1#diff-1622fcab42cac2f075bbe401c3a9bd221ffb7757a029ccd207871bd12fd522b7

But that has been fixed in openapi-schema-validator 0.6.1 https://github.com/python-openapi/openapi-schema-validator/releases/tag/0.6.1 that was released two days ago, so rerunning now will fix that.

But that is all in the Run unittests step, whereas the first run failed in the Upload to Codecov step, so the original problem might still be there. But rerun and we will see

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #1041 (12da4c6) into master (1d341d7) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1041      +/-   ##
==========================================
+ Coverage   86.15%   86.27%   +0.11%     
==========================================
  Files          58       58              
  Lines        5736     5740       +4     
==========================================
+ Hits         4942     4952      +10     
+ Misses        794      788       -6     
Flag Coverage Δ
unittests 86.27% <100.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
tests/boto_tests/test_credentials.py 98.22% <100.00%> (+0.98%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jakkdl
Copy link
Contributor Author

jakkdl commented Sep 25, 2023

🎉

@thehesiod thehesiod self-requested a review September 25, 2023 18:52
@thehesiod thehesiod merged commit 5334722 into aio-libs:master Sep 25, 2023
11 checks passed
@thehesiod
Copy link
Collaborator

ty!

@jakkdl jakkdl deleted the fix_test_load_sso_credentials_without_cache branch September 26, 2023 07:35
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