New Feature: Auto-generation of test code #1817

trevorcampbell · 2023-08-23T21:19:17Z

This PR supercedes #1650 and is compatible with the upcoming 0.9.x release. We can close that one in favour of this new PR, but see the old discussion there and at #1633.

Some notes:

I chose not to rebase the old PR because it was too far out of date.
This new PR has much-improved kernel execution code for instantiating tests compared with the old PR.
There are quite a few new tests implemented to validate AutoTest functionality. But note that AutoTest doesn't modify any UI elements or require any special treatment in the UI. So in the ui-tests I decided to just remove the new problem3.ipynb notebook from the user_guide/source/ps1/ directory during ui-tests to avoid triggering AutoTest.
- I originally tried to include this in the UI tests with the other notebooks, but I really struggled to debug the tests given all the pollution of the logs with existing errors (see this discussion and this issue).

github-actions · 2023-08-23T21:19:50Z

👈 Launch a Binder on branch UBC-DSCI/nbgrader/autotest-0.9.x

brichet

Thanks @trevorcampbell for updating this PR.

In a general case, can it (and should it) be combined with HIDDEN TESTS, to prevent students from knowing the test cases ?

I made some comments/suggestions below.

brichet · 2023-08-28T16:14:57Z

nbgrader/apps/quickstartapp.py

Would it be possible to use the autotests only if a flag is provided (e.g. nbgrader quickstart --autotests) ?

Currently the quickstart generate ps1/Problem3.ipynb, which is a copy of the ps1/Problem1.ipynb with autotests. It can be confusing. We could generate only one of these example, depending on the flag above.
Same for the tests.yml file.

Currently the quickstart generate ps1/Problem3.ipynb, which is a copy of the ps1/Problem1.ipynb with autotests. It can be confusing.

I agree. But the point of this example was to show something identical to problem1.ipynb except implemented using Autotests (so instructors can compare the manual one to the automatic one). But having it named problem3.ipynb is indeed confusing.

Maybe I can create ps1_autotests/ as a new assignment entirely, and make it clear in the documentation that it's a duplicated assignment except implemented using automatic tests?

Would it be possible to use the autotests only if a flag is provided (e.g. nbgrader quickstart --autotests) ?

I suspect if we make it clearer that it's meant to duplicate ps1 but show off autotest functionality, we don't need a separate flag for it.

Thoughts?

I like the idea of the quickstart generating an assignment as little as possible.
My opinion is that autotests is an advanced feature, and bringing a new configuration file (autotest.yml) in the quickstart can be confusing for people who just try to understand the basics of nbgrader.

cc. @jhamrick @BertR @perllaghu if you have an opinion on it

@brichet I'm happy to do as you suggest -- you're right, the quickstart should produce a very simple example to start from, and the flags can be used for advanced users.

nbgrader/docs/source/user_guide/source/ps2/problem.ipynb

nbgrader/docs/source/user_guide/tests.yml

nbgrader/preprocessors/clearsolutions.py

nbgrader/preprocessors/instantiatetests.py

nbgrader/docs/source/user_guide/tests.yml

trevorcampbell · 2023-08-28T22:44:46Z

Thanks for the comments @brichet !

Re: hidden tests -- yes, autotest works as expected with hidden tests. In generate_assignment, the InstantiateTests preprocessor runs before ClearHiddenTests -- once the tests are instantiated, the notebook is processed just like any other notebook.

So far I have done the following, per your request:

renamed tests.yml to autotests.yml
made hash and success optional
avoid running the cell preprocessor if an autotest_delimiter is found but autotests.yml cannot be loaded. This raises an exception to avoid silently missing tests that instructors intended to insert.
fixed warning message string interpolation for logging messages

Two remaining things:

Regarding problem3.ipynb: see my comment above -- the intention was to show users a duplicate assignment implemented using autotests (so they can compare them). Would it work for you if I made a new assignment called ps1_autotests (I'm not sure I like that folder name, but something like that) instead of adding an --autotests flag?
Regarding R: We need both R and Py in our course at UBC, so I would like to keep the R stuff if possible. If not I can separate ps2 into its own PR, but I still think we should keep the code_stub change at the very least.

Let me know what you think about these two, and I'll get that sorted as quickly as possible.

trevorcampbell · 2023-08-28T23:33:40Z

(NB @brichet -- the CI failure isn't a bug in autotest, it's related to the docs build being rate-limited by GitHub. See #1822 )

brichet · 2023-08-29T08:35:06Z

Regarding R: We need both R and Py in our course at UBC, so I would like to keep the R stuff if possible. If not I can separate ps2 into its own PR, but I still think we should keep the code_stub change at the very least.

I agree about keeping the code_stub change.
About the courses in Python and R together, it's related to the use case of UBC.
I'm more in favor of a new PR to select the language(s) in the quickstart using a flag. It could also generate multiple assignments using different languages, using multiple flags.

brichet

Thanks for your quick update @trevorcampbell.
I added a few comments about syntax.

I didn't find documentation about the YAML file ?
Can you add some (if missing), maybe in advanced topic, with a link to it in “Autograder tests” cells with automatically generated test code.

nbgrader/preprocessors/instantiatetests.py

nbgrader/tests/apps/files/autotest-multi-unchanged.ipynb

brichet · 2023-08-29T12:47:48Z

(NB @brichet -- the CI failure isn't a bug in autotest, it's related to the docs build being rate-limited by GitHub. See #1822 )

Yesterday I merged a few changes concerning the documentation into #1819, I don't know if this improves the duration but the following test took less than 12 minutes.
Maybe you can try rebasing on it.

…sts.yml file

Co-authored-by: Nicolas Brichet <[email protected]>

…gnments docs

… -> ps1; add quickstart test for autotest

…ests

trevorcampbell · 2023-08-30T01:09:45Z

OK @brichet :

I rebased on main
I wrote a new documentation section in the Advanced Topics area that details autotests.yml and more advanced parts of autotest in general. Cross linked with the initial section in the Creating and Grading Assignments.
The quickstart app now accepts an --autotest flag. Without the flag, it reproduces only the old ps1 problem set. With the flag, it produces the same looking problem set, except ps1/problem1.ipynb has autotests in it. There is no ps2 any more.
added tests for the --autotest flag in the quickstart tests file
The R ps2 assignment is now removed from the source/ in docs
Added Jinja2 >= 3.0 to pyproject.toml (it was missing before -- autotest needs jinja for template substitution)
Two additional changes:
- fixed a bug: the kernel wasn't running in the right working directory before
- minor improvement: the hashed autotest salts are now deterministic (so multiple calls to nbgrader generate_assignment should generate the same release notebook)

I think that was everything from your suggestions. Let me know what you think!

Set kernel working directory properly

brichet

Thanks again @trevorcampbell for being so responsive.
The documentation looks great, and I like the --autotest option in the quickstart.

Let me know if you think it's ready to merge, it looks good to me.

pyproject.toml

Co-authored-by: Nicolas Brichet <[email protected]>

trevorcampbell · 2023-08-31T22:18:20Z

Thanks @brichet -- FYI: I added two more tests (just to catch the kernel working directory behaviour and release notebook consistency).

I think it's ready to merge now.

…pyter#1817) * initial conversion of autotest code from 0.7.x to 0.9.x * added docs html files for new problems to .gitignore * minor bugfix: shortened --source_with_tests flag in tests * output for source_with_tests set to the right directory * simplified kernel execution code * store kernel name in resources * minor ed * remove problem3.ipynb from the ui-tests * removed commented out timestamp code from earlier snippet execution * tests.yml -> autotests.yml * logging string interpolation * optional success code (and fix execute success code bug) * reraise filenotfound error when find autotest directive but no autotests.yml file * hash optional; raise error if not set * minor comment rephrasing * better if statement formatting Co-authored-by: Nicolas Brichet <[email protected]> * remove unnecessary spaces in start new kernel arg Co-authored-by: Nicolas Brichet <[email protected]> * clear outputs from test notebook * added jinja to pyproject * initial commit of advanced topics docs, minor polish on creating assignments docs * done advanced docs section on autotest * moved problem3 to it's own problem set ps1_autotest * moved problem3 to its own ps1_autotest assignment * remove problem3 from ps1 * ignore autotest assignment files; make quickstart rename ps1_autotest -> ps1; add quickstart test for autotest * remove ps2 R assignment; check for autotest directive in quickstart tests * minor code tags syntax fix in docs * set kernel working dir to notebook resources path * Convert to deterministic salt computed using cell source/index * minor ed * remove extra jinja from pyproject Co-authored-by: Nicolas Brichet <[email protected]> * add tests for kernel working dir and release nb consistency * minor bugfix in workingdir tests --------- Co-authored-by: Nicolas Brichet <[email protected]>

trevorcampbell mentioned this pull request Aug 23, 2023

New Feature: Automatic generation of test code #1650

Closed

brichet added the enhancement label Aug 25, 2023

brichet reviewed Aug 28, 2023

View reviewed changes

brichet reviewed Aug 29, 2023

View reviewed changes

nbgrader/preprocessors/instantiatetests.py Outdated Show resolved Hide resolved

nbgrader/preprocessors/instantiatetests.py Outdated Show resolved Hide resolved

nbgrader/tests/apps/files/autotest-multi-unchanged.ipynb Outdated Show resolved Hide resolved

trevorcampbell added 15 commits August 29, 2023 08:57

initial conversion of autotest code from 0.7.x to 0.9.x

a7bed7e

added docs html files for new problems to .gitignore

19b089e

minor bugfix: shortened --source_with_tests flag in tests

6705227

output for source_with_tests set to the right directory

d823f3e

simplified kernel execution code

48837d9

store kernel name in resources

f0140fc

minor ed

924a26d

remove problem3.ipynb from the ui-tests

9ea6577

removed commented out timestamp code from earlier snippet execution

5a521ac

tests.yml -> autotests.yml

a8ab39f

logging string interpolation

50e40c6

optional success code (and fix execute success code bug)

be7f998

reraise filenotfound error when find autotest directive but no autote…

ec63545

…sts.yml file

hash optional; raise error if not set

e8c3d7e

minor comment rephrasing

2ceab61

trevorcampbell force-pushed the autotest-0.9.x branch from 98c2927 to 2ceab61 Compare August 29, 2023 15:57

trevorcampbell and others added 5 commits August 29, 2023 09:03

better if statement formatting

c53a319

Co-authored-by: Nicolas Brichet <[email protected]>

remove unnecessary spaces in start new kernel arg

52ae394

Co-authored-by: Nicolas Brichet <[email protected]>

clear outputs from test notebook

577c733

added jinja to pyproject

702d10d

initial commit of advanced topics docs, minor polish on creating assi…

da8c972

…gnments docs

trevorcampbell added 6 commits August 29, 2023 14:33

done advanced docs section on autotest

db9b410

moved problem3 to it's own problem set ps1_autotest

a881d2b

moved problem3 to its own ps1_autotest assignment

1af6da2

remove problem3 from ps1

0ca7287

ignore autotest assignment files; make quickstart rename ps1_autotest…

cf1d605

… -> ps1; add quickstart test for autotest

remove ps2 R assignment; check for autotest directive in quickstart t…

4b76b89

…ests

trevorcampbell added 5 commits August 29, 2023 18:24

minor code tags syntax fix in docs

2702e2c

set kernel working dir to notebook resources path

6964065

Merge pull request #19 from UBC-DSCI/set-kernel-resource-directory

cb09c37

Set kernel working directory properly

Convert to deterministic salt computed using cell source/index

d271f9c

minor ed

c58646d

brichet reviewed Aug 31, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

trevorcampbell and others added 3 commits August 31, 2023 10:19

remove extra jinja from pyproject

cc5acbf

Co-authored-by: Nicolas Brichet <[email protected]>

add tests for kernel working dir and release nb consistency

05570a0

minor bugfix in workingdir tests

865f418

brichet merged commit 61290bf into jupyter:main Sep 1, 2023

brichet changed the title ~~New Feature: Auto-generation of test code (compatible with 0.9.x)~~ New Feature: Auto-generation of test code Sep 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Feature: Auto-generation of test code #1817

New Feature: Auto-generation of test code #1817

trevorcampbell commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

brichet left a comment •

edited

Loading

brichet Aug 28, 2023

trevorcampbell Aug 28, 2023 •

edited

Loading

brichet Aug 29, 2023

trevorcampbell Aug 29, 2023

trevorcampbell commented Aug 28, 2023

trevorcampbell commented Aug 28, 2023

brichet commented Aug 29, 2023 •

edited

Loading

brichet left a comment

brichet commented Aug 29, 2023

trevorcampbell commented Aug 30, 2023 •

edited

Loading

brichet left a comment •

edited

Loading

trevorcampbell commented Aug 31, 2023

New Feature: Auto-generation of test code #1817

New Feature: Auto-generation of test code #1817

Conversation

trevorcampbell commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

brichet left a comment • edited Loading

Choose a reason for hiding this comment

brichet Aug 28, 2023

Choose a reason for hiding this comment

trevorcampbell Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

brichet Aug 29, 2023

Choose a reason for hiding this comment

trevorcampbell Aug 29, 2023

Choose a reason for hiding this comment

trevorcampbell commented Aug 28, 2023

trevorcampbell commented Aug 28, 2023

brichet commented Aug 29, 2023 • edited Loading

brichet left a comment

Choose a reason for hiding this comment

brichet commented Aug 29, 2023

trevorcampbell commented Aug 30, 2023 • edited Loading

brichet left a comment • edited Loading

Choose a reason for hiding this comment

trevorcampbell commented Aug 31, 2023

brichet left a comment •

edited

Loading

trevorcampbell Aug 28, 2023 •

edited

Loading

brichet commented Aug 29, 2023 •

edited

Loading

trevorcampbell commented Aug 30, 2023 •

edited

Loading

brichet left a comment •

edited

Loading