aio-libs / aiobotocore

asyncio support for botocore library using aiohttp
https://aiobotocore.aio-libs.org
Apache License 2.0
1.19k stars 183 forks source link

Fix test_load_sso_credentials_without_cache #1041

Closed jakkdl closed 1 year ago

jakkdl commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

jakkdl commented 1 year ago

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): https://github.com/python-jsonschema/jsonschema/compare/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[bot] commented 1 year ago

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%) :arrow_up:

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%) :arrow_up:

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

jakkdl commented 1 year ago

:tada:

thehesiod commented 1 year ago

ty!