search

dandi / redirector

Apache License 2.0
0 stars 2 forks source link

Add tests from dandi-cli #30

Closed jwodder closed 4 years ago

jwodder commented 4 years ago

Closes #29

yarikoptic commented 4 years ago

Oh, sorry I was not 100% clear. I meant to just pip install dandi; python -m purest -v dandi.... as eg an additional step in the testing workflow. Copying them over like this wouldn't be really the point.

See eg https://github.com/datalad/datalad/blob/master/.github/workflows/test_extensions.yml on how we do it in datalad for extensions although we run all tests without selection. May be here it could also be just all since they don't take that long anyways

jwodder commented 4 years ago

There still needs to be some sort of integration between dandi-cli and redirector when testing so that the requests made by the dandi-cli tests go through the version of the redirector in the repository rather than the redirector currently installed on dandiarchive.org. Or do you want dandi-cli to be tested against the redirector on dandiarchive.org?

yarikoptic commented 4 years ago

Ideally our dandi cli tests try on URLs to both the main deployment and docker deployed redirector. ATM iirc the test which started to fail just tests urls to the official deployment.

jwodder commented 4 years ago

So you just want a workflow that installs dandi-cli and runs (a subset of) its tests? What (if any) sort of integration should there be between dandi-cli and the version of redirector in the commit the workflow runs on? Unless dandi-cli is communicating somehow with the local version of redirector, the tests won't be actual tests of redirector, and thus I don't see the point of putting them in the redirector repository.

yarikoptic commented 4 years ago

Yes, indeed, it needs to test against this version of redirector... Could be a custom/patched compose recipe which won't use redirector image from the hub, but build image from the one here. I would've probably made fixture code in client react on some environment variable and tune that compose script before running compose up... May be there is a way to embed conditional logic into compose script - I haven't looked

jwodder commented 4 years ago

Closing in favor of #33.