Skip to content

3091 use testbook to automate notebook testing#3188

Merged
timothy-nunn merged 6 commits intomainfrom
3091-use-nbmake-or-testbook-to-automate-notebook-testing
Jun 3, 2024
Merged

3091 use testbook to automate notebook testing#3188
timothy-nunn merged 6 commits intomainfrom
3091-use-nbmake-or-testbook-to-automate-notebook-testing

Conversation

@clmould
Copy link
Copy Markdown
Collaborator

@clmould clmould commented May 22, 2024

Description

Checklist

I confirm that I have completed the following checks:

  • I have justified any large differences in the regression tests caused by this pull request in the comments.
  • I have added new tests where appropriate for the changes I have made.
  • If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments.
  • If I have made documentation changes, I have checked they render correctly.
  • I have added documentation for my change, if appropriate.

@clmould clmould linked an issue May 22, 2024 that may be closed by this pull request
@clmould clmould requested a review from timothy-nunn May 22, 2024 16:11
@clmould clmould force-pushed the 3091-use-nbmake-or-testbook-to-automate-notebook-testing branch from 704f7f1 to a73cdaf Compare May 23, 2024 14:35
@clmould clmould force-pushed the 3091-use-nbmake-or-testbook-to-automate-notebook-testing branch from a73cdaf to 9b3c8bb Compare June 3, 2024 10:46
@clmould clmould force-pushed the 3091-use-nbmake-or-testbook-to-automate-notebook-testing branch from 9b3c8bb to 1e9ab71 Compare June 3, 2024 15:48
Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Clair,

Good to have so much duplication removed. The plot proc assets arent being deleted and there is a print statement that shouldnt be there. Other than that, good to go.

Tests pass locally, quirk of our CI means integration tests dont pass on the feature branch CI, as expected.

Tim

Comment thread tests/integration/test_examples.py Outdated
assert check_positive
with testbook("csv_output.ipynb", execute=True):
# Check csv file is created
print(os.getcwd())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a logger or removed

Comment thread tests/integration/test_examples.py Outdated
Comment on lines +47 to +49
yield
plot_proc_1 = Path("../examples/plot_proc_1")
plot_proc_2 = Path("../examples/plot_proc_2")
plot_proc_3 = Path("../examples/plot_proc_3")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing .png?

@clmould clmould requested a review from timothy-nunn June 3, 2024 16:22
@timothy-nunn timothy-nunn merged commit 6cd9ccc into main Jun 3, 2024
@timothy-nunn timothy-nunn deleted the 3091-use-nbmake-or-testbook-to-automate-notebook-testing branch June 3, 2024 16:38
@mkovari mkovari restored the 3091-use-nbmake-or-testbook-to-automate-notebook-testing branch June 4, 2024 10:40
mkovari added a commit that referenced this pull request Jun 4, 2024
mkovari added a commit that referenced this pull request Jun 4, 2024
chris-ashe pushed a commit that referenced this pull request Jul 3, 2024
* add testbook to requirements

* starting testbook work

* remove duplicate scan.py script in examples, test that mfile is created from scan.ipynb

* use testbook for examples tests and remove duplicate .py files

* stop timeout warning in test_scan

* corrections from review comments
chris-ashe pushed a commit that referenced this pull request Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use nbmake or testbook to automate notebook testing

2 participants