-
Notifications
You must be signed in to change notification settings - Fork 2
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
NBS Test Cases #123
NBS Test Cases #123
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #123 +/- ##
=======================================
Coverage 97.15% 97.15%
=======================================
Files 30 30
Lines 1301 1301
=======================================
Hits 1264 1264
Misses 37 37 ☔ View full report in Codecov by Sentry. |
7377609
to
aa66f56
Compare
c36a188
to
24e7026
Compare
957bdaf
to
267ef0c
Compare
@cbrinson-rise8 I was going through the README and ran the default tests. I noticed the all of the expected results in the output were either a failure or a match, which is different from the results.csv file you committed. Any ideas why my results would have been different from the ones in the PR? |
Hmm not sure but the output file I committed might just be old. I'll run through running the tests and see what my new output file would look like. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this work! Excited that we'll have the functionality to test results, which paves the way for calculating match accuracy performance metrics!
Left several questions and a few suggestions.
@cbrinson-rise8 I think pretty much all of my previous questions have been resolved. I did push a small change to update some language in the README and the sample tests cases, please take a look to make sure you agree with those. My only remaining thought is I think we should remove tests/algorithm/results/output.csv. That's a generated file and not source, so I think it is best to remove that. Additionally, we may want to add a rule to the .gitignore file, so future devs don't accidentally commit that file. Thoughts? |
@cbrinson-rise8 FYI, I also rebased against main and adjusted the algorithm_configuration.json accordingly. 8e1575d |
54f2c98
to
c0815a1
Compare
c0815a1
to
4000764
Compare
Description
NBS provided us a spreadsheet that defines an existing state and then a bunch of tests with expected results. Architect a testing script of sorts, that can parse a similar spreadsheet (can just be a CSV) with seeding data and iterate through the tests with the expectations.
Related Issues
closes #110
Additional Notes
For a full explanation of run instructions, directory structure, and file uses check out the README.md
The key inputs to these tests will be the following:
The setup should make it easy to edit any 3 of those above files, run the test cases, and examine the results.
<--------------------- REMOVE THE LINES BELOW BEFORE MERGING --------------------->
Checklist
Please review and complete the following checklist before submitting your pull request:
Checklist for Reviewers
Please review and complete the following checklist during the review process: