con / tributors

Pay tribute to your contributors! A tool to automatically update contributor files.
https://con.github.io/tributors/
Apache License 2.0
13 stars 5 forks source link

Create list of tests needed #26

Open vsoch opened 4 years ago

vsoch commented 4 years ago

hey @yarikoptic ! I've done just about all the changes / updates that I think are good for this first version, so please have it it / unlease the Yarik! Now is the right time to start review and tell me all the things I did wrong :) In all seriousness I always can quickly put together a first go, but then lots of tweaking / fine tuning / testing is needed. Actually, we don't technically have proper python tests, so if you want to make a list of tests to write I can go from there!

yarikoptic commented 4 years ago

having tests would be great! but meanwhile I will follow up with a few separate "issues" on top of https://github.com/con/tributors/issues/22#issuecomment-653251074 initial experience

vsoch commented 4 years ago

okay! Let's prioritize tests though, since we don't have them (it makes me uneasy).

yarikoptic commented 4 years ago

What about starting with an integration test for sample use cases where sample input is provided and sample output is verified? vcr library then could be used to record any interaction which happen in that initial run (and committed as well, but be careful to not run with secrets in your environment variables)

Then as bugs are fixed - provide unit tests which code the fixed up behavior in corresponding functions

vsoch commented 4 years ago

Good idea - we will want to do this after adding support for some kind of --from or --lookup.

But no more today! Dinnertime!

yarikoptic commented 4 years ago

as requested in #48 I think it would be useful to establish a "high level" unittest for verifying finding out correct ORCID ids.

e.g. for mine should find a single entry ```shell $> curl --silent -X GET --header 'Accept: application/json' 'https://pub.orcid.org/v3.0/expanded-search?q=email:debian@onerussian.com' | jq . { "expanded-result": [ { "orcid-id": "0000-0003-3456-2493", "given-names": "Yaroslav", "family-names": "Halchenko", "credit-name": null, "other-name": [ "Ярослав Олеговіч Гальченко" ], "email": [ "debian@onerussian.com" ], "institution-name": [ "Center for Open Neuroscience", "Dartmouth College", "Debian Project", "New Jersey Institute of Technology", "Rutgers University", "University of New Mexico", "Vinnytsia State Technical University" ] } ], "num-found": 1 } ```
FWIW - it can even do glob matching in email address seems to me giving information that there are 30k records with gmail.com and 121k records with some email address: ```shell $> curl --silent -X GET --header 'Accept: application/json' 'https://pub.orcid.org/v3.0/expanded-search?q=email:*@gmail.com' | jq . | grep num-found "num-found": 30073 $> curl --silent -X GET --header 'Accept: application/json' 'https://pub.orcid.org/v3.0/expanded-search?q=email:*' | jq . | grep num-found "num-found": 121636 ```
vsoch commented 4 years ago

I can add this to the PR! Can you tell me more about what you mean by using the vcr library to "record transactions" (I'm not familiar with this). My basic intuition would be to add a few tests for the list you generated, to be run on a PR, which I don't think would be abusing the API. What tapes?

yarikoptic commented 4 years ago

https://vcrpy.readthedocs.io/en/latest/ FWIW, we use it in datalad for some tests and have helper: https://github.com/datalad/datalad/blob/master/datalad/support/vcr_.py#L44 but most likely you don't need anything like that ;)