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.

Sign up for GitHub

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

Jump to bottom

Re-record VCR tapes using newer versions of libraries #1337

Merged
merged 2 commits into from
Nov 1, 2023
Merged

Re-record VCR tapes using newer versions of libraries #1337

merged 2 commits into from
Nov 1, 2023

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Oct 16, 2023 •
edited
Loading

I believe that somehow some upgrade among

❯ wdiff -3 /tmp/f1 /tmp/f2

====================================================================== [-06/github/cron/20231006T060312/0c1d4a0/Tests/5197/test-]{+07/github/cron/20231007T060245/0c1d4a0/Tests/5198/test+} ======================================================================
 [-dependencies.txt:2023-10-06T06:05:18.6948668Z-] {+dependencies.txt:2023-10-07T06:04:51.1091526Z+}
======================================================================
 [-boto3-1.28.61 botocore-1.31.61-] {+boto3-1.28.62 botocore-1.31.62+}
======================================================================
 [-entrypoints-0.4-]
======================================================================
 [-numcodecs-0.11.0-] {+numcodecs-0.12.0+}
======================================================================
 [-urllib3-1.26.17-] {+urllib3-2.0.6+}
======================================================================

(so likely urllib3) lead to drastically different behavior which triggered VCR to try to not reuse prior tape but then subsequent dialog started to fail -- we got no json. As you can see in the diff we observe that now we do get json (as requested) recorded although previously it was some "binary".

Closes #1332 although I am afraid that we might need to really identify the library lead to breakage and then set explicit minimal version of tests so we have consistent testing across distributions etc

PS pre-commit refused to commit since it kept codespell'ing the .vcr.yaml files although ignored within codespell config of setup.cfg. I guess codespell does not consult that skip config if explicit file names are given. Might need to add those to excludes of pre-commit

@yarikoptic
Re-record VCR tapes using newer versions of libraries
4fe7c39 
I believe that somehow some upgrade among

❯ wdiff -3 /tmp/f1 /tmp/f2

======================================================================
[-06/github/cron/20231006T060312/0c1d4a0/Tests/5197/test-]{+07/github/cron/20231007T060245/0c1d4a0/Tests/5198/test+}
======================================================================
 [-dependencies.txt:2023-10-06T06:05:18.6948668Z-] {+dependencies.txt:2023-10-07T06:04:51.1091526Z+}
======================================================================
 [-boto3-1.28.61 botocore-1.31.61-] {+boto3-1.28.62 botocore-1.31.62+}
======================================================================
 [-entrypoints-0.4-]
======================================================================
 [-numcodecs-0.11.0-] {+numcodecs-0.12.0+}
======================================================================
 [-urllib3-1.26.17-] {+urllib3-2.0.6+}
======================================================================

(so likely urllib3) lead to drastically different behavior which triggered
VCR to try to not reuse prior tape but then subsequent dialog started to
fail -- we got no json.  As you can see in the diff we observe that
now we do get json (as requested) recorded although previously
it was some "binary".
@yarikoptic
Copy link
Member Author

typing is to be fixed in #1336

@codecov
Copy link

codecov bot commented Oct 16, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a0199fc) 88.78% compared to head (25bd01c) 88.94%.
Report is 14 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1337      +/-   ##
==========================================
+ Coverage   88.78%   88.94%   +0.16%     
==========================================
  Files          76       76              
  Lines       10216    10240      +24     
==========================================
+ Hits         9070     9108      +38     
+ Misses       1146     1132      -14
Flag Coverage Δ
unittests 88.94% <100.00%> (+0.16%) ⬆️

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

Files Coverage Δ
dandi/cli/tests/test_service_scripts.py 98.24% <100.00%> (+0.06%) ⬆️

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yarikoptic
Copy link
Member Author

well, this one seems to fix those two runs where the test was failing

Tests / test (ubuntu-latest, 3.10, normal) (pull_request) Successful in 15m
Tests / test (ubuntu-latest, 3.11, normal) (pull_request) Successful in 15m

but now it is failing for 3.8 and 3.9! 
dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandi-cli/2023/10/16/github/pr/1337/4fe7c39$ git grep -H 'FAILED$'
Tests/5215/13_test (ubuntu-latest, 3.8, dandi-api).txt:2023-10-16T18:54:51.8848342Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/13_test (ubuntu-latest, 3.8, dandi-api).txt:2023-10-16T18:54:52.0571627Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/13_test (ubuntu-latest, 3.8, dandi-api).txt:2023-10-16T18:54:52.2139767Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/13_test (ubuntu-latest, 3.8, dandi-api).txt:2023-10-16T18:54:52.3701221Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/13_test (ubuntu-latest, 3.8, dandi-api).txt:2023-10-16T18:54:52.5248098Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/14_test (ubuntu-latest, 3.8, dev-deps).txt:2023-10-16T18:56:07.4628507Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/14_test (ubuntu-latest, 3.8, dev-deps).txt:2023-10-16T18:56:07.6669957Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/14_test (ubuntu-latest, 3.8, dev-deps).txt:2023-10-16T18:56:07.8603079Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/14_test (ubuntu-latest, 3.8, dev-deps).txt:2023-10-16T18:56:08.0578001Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/14_test (ubuntu-latest, 3.8, dev-deps).txt:2023-10-16T18:56:08.2509842Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/15_test (ubuntu-latest, 3.8, nfs).txt:2023-10-16T18:56:36.0445856Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/15_test (ubuntu-latest, 3.8, nfs).txt:2023-10-16T18:56:36.2452680Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/15_test (ubuntu-latest, 3.8, nfs).txt:2023-10-16T18:56:36.4430664Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/15_test (ubuntu-latest, 3.8, nfs).txt:2023-10-16T18:56:36.6349903Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/15_test (ubuntu-latest, 3.8, nfs).txt:2023-10-16T18:56:36.8253267Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/5_test (ubuntu-latest, 3.8, normal).txt:2023-10-16T18:55:59.0782173Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/5_test (ubuntu-latest, 3.8, normal).txt:2023-10-16T18:55:59.2907524Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/5_test (ubuntu-latest, 3.8, normal).txt:2023-10-16T18:55:59.5288766Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/5_test (ubuntu-latest, 3.8, normal).txt:2023-10-16T18:55:59.7533537Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/5_test (ubuntu-latest, 3.8, normal).txt:2023-10-16T18:55:59.9886675Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/6_test (ubuntu-latest, 3.9, normal).txt:2023-10-16T18:55:56.5876078Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/6_test (ubuntu-latest, 3.9, normal).txt:2023-10-16T18:55:56.8783784Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/6_test (ubuntu-latest, 3.9, normal).txt:2023-10-16T18:55:57.1014339Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/6_test (ubuntu-latest, 3.9, normal).txt:2023-10-16T18:55:57.3195470Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/6_test (ubuntu-latest, 3.9, normal).txt:2023-10-16T18:55:57.5276320Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dandi-api)/9_Run Dandi API tests only.txt:2023-10-16T18:54:51.8848306Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dandi-api)/9_Run Dandi API tests only.txt:2023-10-16T18:54:52.0571590Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dandi-api)/9_Run Dandi API tests only.txt:2023-10-16T18:54:52.2139731Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dandi-api)/9_Run Dandi API tests only.txt:2023-10-16T18:54:52.3701176Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dandi-api)/9_Run Dandi API tests only.txt:2023-10-16T18:54:52.5247661Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dev-deps)/7_Run all tests.txt:2023-10-16T18:56:07.4628455Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dev-deps)/7_Run all tests.txt:2023-10-16T18:56:07.6669901Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dev-deps)/7_Run all tests.txt:2023-10-16T18:56:07.8602577Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dev-deps)/7_Run all tests.txt:2023-10-16T18:56:08.0577945Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/test (ubuntu-latest, 3.8, dev-deps)/7_Run all tests.txt:2023-10-16T18:56:08.2509783Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/test (ubuntu-latest, 3.8, nfs)/7_Run all tests.txt:2023-10-16T18:56:36.0445800Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/test (ubuntu-latest, 3.8, nfs)/7_Run all tests.txt:2023-10-16T18:56:36.2452625Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/test (ubuntu-latest, 3.8, nfs)/7_Run all tests.txt:2023-10-16T18:56:36.4430619Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/test (ubuntu-latest, 3.8, nfs)/7_Run all tests.txt:2023-10-16T18:56:36.6349847Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/test (ubuntu-latest, 3.8, nfs)/7_Run all tests.txt:2023-10-16T18:56:36.8253221Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/test (ubuntu-latest, 3.8, normal)/7_Run all tests.txt:2023-10-16T18:55:59.0782130Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/test (ubuntu-latest, 3.8, normal)/7_Run all tests.txt:2023-10-16T18:55:59.2907484Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/test (ubuntu-latest, 3.8, normal)/7_Run all tests.txt:2023-10-16T18:55:59.5288717Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/test (ubuntu-latest, 3.8, normal)/7_Run all tests.txt:2023-10-16T18:55:59.7533483Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/test (ubuntu-latest, 3.8, normal)/7_Run all tests.txt:2023-10-16T18:55:59.9886008Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED
Tests/5215/test (ubuntu-latest, 3.9, normal)/7_Run all tests.txt:2023-10-16T18:55:56.5876019Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
Tests/5215/test (ubuntu-latest, 3.9, normal)/7_Run all tests.txt:2023-10-16T18:55:56.8783727Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
Tests/5215/test (ubuntu-latest, 3.9, normal)/7_Run all tests.txt:2023-10-16T18:55:57.1014284Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
Tests/5215/test (ubuntu-latest, 3.9, normal)/7_Run all tests.txt:2023-10-16T18:55:57.3195418Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
Tests/5215/test (ubuntu-latest, 3.9, normal)/7_Run all tests.txt:2023-10-16T18:55:57.5276228Z dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED

so, for proper fix -- we need to either have version specific tapes (possible but bleh) or just skip on older (even worse)... I will probably add version specific taping unless someone has better ideas

This was referenced Oct 17, 2023
Merged
Merged
@yarikoptic yarikoptic added the tests Add or improve existing tests label Nov 1, 2023
@yarikoptic
Copy link
Member Author

ok, something in vcr or library (requests?) difference between earlier 3.8 and 3.9 causes binary instead of updated format. I do not think it is worth fighting this battle -- I will just announce this test xfail on those older pythons and be done for now.

@yarikoptic yarikoptic merged commit b575ef5 into master Nov 1, 2023
23 of 25 checks passed
@yarikoptic yarikoptic deleted the bf-vcr2 branch November 1, 2023 01:26
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 1, 2023

🚀 PR was released in 0.57.0 🚀

yarikoptic added a commit to dandi/dandi-archive that referenced this pull request Nov 1, 2023
@yarikoptic
Revert "Skip broken dandi-cli tests"
8a5cfcc 
This reverts commit 56969e5.

That test was adjusted for becoming compatible with recent python/vcr and
marked xfailed on older ones. See
dandi/dandi-cli#1337
which was released in dandi-cli 0.57.0 .

As such, custom skip is no longer needed here.
@yarikoptic yarikoptic mentioned this pull request Nov 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released tests Add or improve existing tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test_update_dandiset_from_doi started to fail
1 participant
@yarikoptic