ice
ice copied to clipboard
Running test recipes in README
The README suggests in https://github.com/oughtinc/ice#running-tests:
Cheap integration tests:
./scripts/run-recipe.sh --mode test
This had me confused for a while.
- This prompts me select a recipe, which is already surprising. I expected it to automatically pick parameters for testing, I don't know which recipe I should select.
- The first few choices in the list all require papers, so selecting any of them gives a long traceback ending in
TypeError: missing a required argument: 'paper'which looks more like a fundamental bug than incorrect user inputs. - Once I figured out that I needed to specify other arguments to pass papers, it wasn't clear what those arguments should be. The CLI help doesn't offer any example values. Maybe the README could suggest something like:
./scripts/run-recipe.sh --mode test --recipe-name AdherenceKeywordBaseline --input-files ./papers/keenan-2018-tiny.txt
- I see this relevant-looking comment:
https://github.com/oughtinc/ice/blob/5ee41613413c7e214a5ea057ea0166b64cfc7c34/main.py#L151-L153
Possible approaches for dealing with this, which I could do if you want:
- Prompt for papers if the recipe takes them.
- Filter the list of recipes to choose from based on whether or not papers are specified via CLI args.
- Show a more helpful error message without the long traceback.
- Running with those arguments gives output ending abruptly in a title saying
Final result for keenan-2018-tiny.txtwith nothing underneath. Where are these results of which it speaks? Did the test pass? Should I use different arguments? - Running the unit tests with
./scripts/run-tests.shruns all recipes with and without papers automatically.- Why do the 'unit tests' apparently include 'integration tests'?
- Why suggest
run-recipe.shifrun-tests.shtakes care of this already?
I agree that this is pretty confusing. Most of the scripts have pretty sharp edges and I'd like to revise some of them and hopefully get rid of others. As a stopgap, I have an upcoming change that renames this section of the README to "Advanced command line usage".
For these scripts in particular, I'll determine priority next week once the upcoming ICE release is done.