sherlock
sherlock copied to clipboard
Improved Testing and More Modular
Primary purpose:
- Move tests outside of the module as this is more common. Also means if Sherlock is made into a package then tests won't be included. Requires some changes to code to make it more modular (i.e. using fully qualified references)
- Add False negative/positive tests to monitor progress of these (as they seem the most common issue reported). Help ensure that any future PRs that "accidentally" fix them are aware and update the issues reported.
Also:
- Moved to using pytest. I find it results in shorter tests and more intuitive - hopefully you'll agree.
- Add Pipfile(.lock) to support
pipenv
use, demonstrated this by alteringpull_request
workflow to use this.
I re-ran the tests and got the same results (including the coverage tests). It takes roughly 10m to run all tests locally but markers are used to help omit the expensive coverage tests if desired. Please note that there were a number of issues for false negatives or positives that I could not include. I'm unsure if they should be closed/updated etc but they fall into the following categories:
- Fixed: actual result is now what we expected;
- #1074
- #989
- #973
- #960
- #959
- #958
- #904
- #913 (all 3 mentioned)
- Missing information
- #883 - site not specified
- #901 - site not specified
- #961 - username not specified
- #962 - username not specified
- #941 - username not specified (also seems platform dependent and WSL2 may be okay)
- Related to Rate Limiting
- #816 - may want to remove the false negative flag as this seems to be a feature request on avoiding rate limiting issues.
The following false neg/pos-itives are included:
- #806
- #988
- #1034
- #1067
- #1068
- #1069
- #1070
- #1071
- #1072
- #1073
- #1075
- #1076
- #1077
Hi,
I am not sure who the maintainers are of this project but I'd like to help. If this PR is okay then I can do further work around more modularity/testing. Also happy to review other PRs if that helps too.
Regards, Louis
Hello @lwdunnill , Nice work and sorry for late response.
I like what you've done so far.
- but I don't think it's good two different "test" directory in different places. let's keep them in the same place.
- Also it's good to test the coverage, but I think it's better to use
data.json
file itself for testing false positives. - In general, It would be better to have small PRs instead of Huge multi-purpose PRs, which is hard to follow and review.
So far if I don't understand you approach let me know, we can discuss it.