-
Notifications
You must be signed in to change notification settings - Fork 4
test: move example test inputs to json files #411
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #411 +/- ##
==========================================
- Coverage 76.62% 76.37% -0.26%
==========================================
Files 29 29
Lines 3359 3420 +61
Branches 525 533 +8
==========================================
+ Hits 2574 2612 +38
- Misses 553 576 +23
Partials 232 232 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
should we call these empty_input.json or handle in a different way (probably partially defeats point of refactor but more explicit)?
|
Thanks @jpbrodrick89. This looks like a sensible first step. Wrt the questions you raise, I wonder if a "test case" structure like this would make sense: {
"inputs": { }
"expected_result": {
"status_code": 400,
"outputs": { }
},
"endpoint": "apply"
}Unfortunately you can't use file aliasing with this, but almost: (untested) $ tesseract run mytess apply | jq '.[inputs]' testcase.json... and perhaps to be augmented with a dedicated What do people think? |
|
Good idea, @dionhaefner, I'm in two minds but I think this is probably the right way to go. There are challenges we need to think carefully about but these probably apply regardless of the json structure. Advantages of single json
Disadvantages of single json
General challenges of both approaches What exactly are we trying to achieve here? Just regression testing or a framework for more general testing of Do we want to support "fuzzy tests", e.g. allow for slight floating point differences, pytest-regressions mainly handles this by rounding for Do we want to check against all output formats, always do one or make this customisable? Will we deprecate our "contains" tests in lieu of the new full regressions. Do we want to reuse the apply inputs automatically in gradient endpoints, then extending them with AD fields? |
|
I care less about the Tesseract Core test suite than providing a general way to test user-built Tesseracts. I'd expect those to live in repos with Makefiles and build scripts, not necessarily full-fledged Python packages covered by |
Relevant issue or PR
examples/folder #61Description of changes
This PR just migrates the endpoints INPUTS used in test_examples.py and otherwise leaves tests untouched. These are all stored in the
tesseract_apifolder in atest_casesfolder. The naming convention is{descriptor:camelCase}_{endpoint}_input.jsonexcept for the apply endpoint where the convention is just{descriptor:camelCase}_inputs.json.Follow-up work will come in two stages (@dionhaefner lmk if you'd prefer this to be one PR or a sequence of PRs):
tesseract test/regress/test-regressCLI command to allow users to leverage such functionality.Optional (could come at any stage): Introduce
tesseract testgencommand to allow users to automate step 2 above.Critical concern
The default base64 array encoding is not human-readable and may make tests hard to reason about without additional diagnostic efforts. Potential solutions include always using list format in CI tests (less exhaustive than currently) or we have three json files one for each encoding (but then we can't guarantee they stay synced), or we use list but generate other json+... encodings on the fly (probably the best option if we think human readability is crucial).
Testing done