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

Adjust tests for the added email requirement for contact person #1438

Merged
merged 3 commits into from
May 3, 2024

Conversation

candleindark
Copy link
Member

This PR updates SampleDandisetFactory used for testing to meet the requirement imposed by dandi-schema of having email for a contributor who is a contact person, dandi/dandi-schema#235.

Additionally, it sets DJANGO_DANDI_DEV_EMAIL needed for running the latest dandiarchive/dandiarchive-api image in testing.

This environment var is needed for running the latest
`dandiarchive/dandiarchive-api` image
This update is for meeting the requirement of having
email for a contributor who is a contact person
imposed by dandischema,
dandi/dandi-schema#235
@candleindark candleindark marked this pull request as draft May 1, 2024 20:35
Copy link

codecov bot commented May 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.63%. Comparing base (15196a9) to head (bacef78).
Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1438      +/-   ##
==========================================
+ Coverage   88.54%   88.63%   +0.08%     
==========================================
  Files          77       77              
  Lines       10561    10563       +2     
==========================================
+ Hits         9351     9362      +11     
+ Misses       1210     1201       -9     
Flag Coverage Δ
unittests 88.63% <ø> (+0.08%) ⬆️

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

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

@candleindark
Copy link
Member Author

candleindark commented May 1, 2024

The current test failures in Ubuntu systems may be related to the anys library, but the error message can be misleading. It can very well be something else.

FAILED dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] - AssertionError: assert {'@context': ...6/draft', ...} == {'@context': .../draft'), ...}
  
  Omitting 18 identical items, use -vv to show
  Differing items:
  {'contributor': [{'email': 'nemo@example.com', 'name': 'Tests, Dandi-Cli', 'roleName': ['dcite:Author', 'dcite:Contact...{'affiliation': [], 'identifier': '0000-0002-5366-8501', 'includeInCitation': True, 'name': 'Goard, Michael J.', ...}]} != {'contributor': [{'name': 'Tests, Dandi-Cli', 'roleName': ['dcite:Author', 'dcite:ContactPerson'], 'schemaKey': 'Perso...{'affiliation': [], 'identifier': '0000-0002-5366-8501', 'includeInCitation': True, 'name': 'Goard, Michael J.', ...}]}
  
  Full diff:
    {
        '@context': 'https://raw.githubusercontent.com/dandi/schema/master/releases/0.6.7/context.json',
        'access': [
            {
                'schemaKey': 'AccessRequirements',
                'status': 'dandi:OpenAccess',
            },
        ],
        'assetsSummary': {
            'numberOfBytes': 0,
            'numberOfFiles': 0,
            'schemaKey': 'AssetsSummary',
        },
  -     'citation': AnyFullmatch('Sit,\\ Kevin\\ K\\.;\\ Goard,\\ Michael\\ J\\.\\ \\(\\d{4}\\)\\ Coregistration\\ of\\ heading\\ to\\ visual\\ cues\\ in\\ retrosplenial\\ cortex\\ \\(Version\\ draft\\)\\ \\[Data\\ set\\]\\.\\ DANDI\\ archive\\.\\ http://localhost:8085/dandiset/000006/draft'),
  +     'citation': 'Sit, Kevin K.; Goard, Michael J. (2024) Coregistration of heading to '
  +     'visual cues in retrosplenial cortex (Version draft) [Data set]. DANDI '
  +     'archive. http://localhost:8085/dandiset/000006/draft',
        'contributor': [
            {
  +             'email': 'nemo@example.com',
                'name': 'Tests, Dandi-Cli',
                'roleName': [
                    'dcite:Author',
                    'dcite:ContactPerson',
                ],
                'schemaKey': 'Person',
            },
            {
                'affiliation': [],
                'includeInCitation': True,
                'name': 'Sit, Kevin K.',
                'roleName': [
                    'dcite:Author',
                ],
                'schemaKey': 'Person',
            },
            {
                'affiliation': [],
        'wasGeneratedBy': [
            {
                'description': 'Metadata (contributor, name, description, relatedResource) was '
                'enhanced with data from DOI 10.1038/s41467-023-37704-5 by DANDI '
                'cli',
  -             'endDate': ANY_AWARE_DATETIME_STR,
  -             'id': AnyFullmatch('urn:uuid:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}'),
  +             'endDate': '2024-05-01 21:15:57.505467+00:00',
  +             'id': 'urn:uuid:43a99fde-2978-426f-be06-03029406b131',
                'name': 'Metadata extraction from DOI',
                'schemaKey': 'Activity',
  -             'startDate': ANY_AWARE_DATETIME_STR,
  +             'startDate': '2024-05-01 21:15:57.416761+00:00',
                'wasAssociatedWith': [
                    {
                        'identifier': 'RRID:SCR_019009',
                        'name': 'DANDI Command Line Interface',
                        'schemaKey': 'Software',
                        'url': 'https://github.com/dandi/dandi-cli',
                        'version': '0.61.2+18.gc00b6748',
                    },
                ],
            },
        ],
    }
dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1101/2020.01.17.909838-biorxiv] FAILED
dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1523/JNEUROSCI.6157-08.2009-jneurosci] FAILED
dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1016/j.neuron.2019.10.012-neuron] FAILED
dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.7554/eLife.48198-elife] FAILED
dandi/cli/tests/test_service_scripts.py::test_update_dandiset_from_doi[10.1038/s41467-023-37704-5-nature] FAILED

@yarikoptic
Copy link
Member

isn't it again just due to added email': 'nemo@example.com', which now should be in expected entry of contributor?

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "sed -i -e 's/^\\( *\\)\\(\"name\": \"Tests, Dandi-Cli\",\\)/\\1\\2\\n\\1\"email\": \"nemo@example.com\",/g' dandi/cli/tests/data/update_dandiset_from_doi/biorxiv.json dandi/cli/tests/data/update_dandiset_from_doi/elife.json dandi/cli/tests/data/update_dandiset_from_doi/jneurosci.json dandi/cli/tests/data/update_dandiset_from_doi/nature.json dandi/cli/tests/data/update_dandiset_from_doi/neuron.json",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Member

yeap -- I git grep -2 'Tests, Dandi-Cli' to see where that record is also used, saw those target .json files and modified them all in a sed invocation I am now pushing . That test is passing now.

@yarikoptic
Copy link
Member

ok -- now we have only 3.12s failing due to

FAILED dandi/tests/test_files.py::test_validate_simple1 - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_files.py::test_validate_simple1_no_subject - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_files.py::test_validate_simple2 - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_files.py::test_validate_simple2_new - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_files.py::test_validate_simple3_no_subject_id - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_files.py::test_validate_bogus - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_validate.py::test_validate_nwb_error - ModuleNotFoundError: No module named 'distutils'
FAILED dandi/tests/test_validate.py::test_validate_bids[ieeg_epilepsyNWB] - ModuleNotFoundError: No module named 'distutils'

in

dandi/files/bases.py:512: in get_validation_errors
    from nwbinspector import Importance, inspect_nwbfile, load_config
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/nwbinspector/__init__.py:3: in <module>
    from .nwbinspector import inspect_all, inspect_nwbfile, inspect_nwbfile_object, run_checks, load_config
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/nwbinspector/nwbinspector.py:15: in <module>
    from distutils.util import strtobool
E   ModuleNotFoundError: No module named 'distutils'

which is indeed still in use! https://github.com/NeurodataWithoutBorders/nwbinspector/blob/dev/src/nwbinspector/testing.py#L6

filed

but I think we are done here, taking out of draft and merging.

@yarikoptic yarikoptic changed the title Update test utility for the email requirement for contact person Adjust tests for the added email requirement for contact person May 3, 2024
@yarikoptic yarikoptic added patch Increment the patch version when merged release Create a release when this pr is merged labels May 3, 2024
@yarikoptic yarikoptic marked this pull request as ready for review May 3, 2024 13:56
@yarikoptic
Copy link
Member

now 100% certain if release would workout since we would still fail on 3.12 due to nwbinspector, but let's try

@yarikoptic yarikoptic merged commit d67e2ea into dandi:master May 3, 2024
23 of 26 checks passed
Copy link

github-actions bot commented May 3, 2024

🚀 PR was released in 0.62.0 🚀

@candleindark candleindark deleted the contact-person-email branch May 3, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants