Skip to content
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

Add a test to CI for checking docker build and tutorial notebooks #652

Merged
merged 30 commits into from
Jun 12, 2023

Conversation

caleb-johnson
Copy link
Collaborator

@caleb-johnson caleb-johnson commented Jun 7, 2023

Summary

Add a test to CI for testing the jupyter build and the tutorial notebooks on pushes to main

@psschwei
Copy link
Collaborator

psschwei commented Jun 7, 2023

Seems like the test failure is when trying to connect to the gateway service.

It might be worth adding a step to dump the logs on failure... something like:

- name: dump logs on failure
  if: ${{ failure() }}
  run: |
    docker compose logs gateway
    docker compose logs jupyter
    ...

@psschwei
Copy link
Collaborator

psschwei commented Jun 9, 2023

It might be worth just running the getting started notebooks in the test, as those should let us know whether the basic functionality (setting up a provider, submitting a program, getting results) is working or not.

@caleb-johnson
Copy link
Collaborator Author

caleb-johnson commented Jun 9, 2023

It might be worth just running the getting started notebooks in the test, as those should let us know whether the basic functionality (setting up a provider, submitting a program, getting results) is working or not.

I see where you're coming from, but I think if we build everything, we might as well make sure all our docs will actually run for the users. This is kind of like a docker build test + a notebooks test in my mind.

I'm planning on ignoring the ElectronicStructureProblem notebook for now. The Nature code in there is badly deprecated and about to be broken soon. It needs a total refactor.

There is also a strange ray visualization in 07_working_with_large_datasets -- ray.data.from_items(...).show(1) -- that is breaking only in CI (not locally). I'm going to comment that out and see if I can get these tests passing, and then we can decide where to go from there with these couple of straggler notebooks

run: sleep 90
- name: Test notebooks in the docker environment
shell: bash
run: docker exec qs-jupyter "bash" "-c" "pip install nbmake pytest && pytest --nbmake --ignore=/home/jovyan/serverless/guides/07_working_with_datasets.ipynb --ignore=/home/jovyan/serverless/examples/06_electronic_structure_problem.ipynb /home/jovyan/serverless/"
Copy link
Collaborator Author

@caleb-johnson caleb-johnson Jun 9, 2023

Choose a reason for hiding this comment

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

We will ignore two tests here.

#671 PR will handle the ESProblem notebook
#672 explains why we will ignore the dataset notebook here to start. I will follow up with @IceKhan13 about why Ray is behaving like this in CI and address this issue in a separate PR

@caleb-johnson
Copy link
Collaborator Author

For some reason, when I go to old commits in which this docker test pased, there is no sign of those tests being run. Kind of strange... I suppose you'll have to take my word this test passes the CI? :)

Copy link
Collaborator

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

LGTM - kicked this test again just to see, but once it's done I think we're good to merge this

@caleb-johnson caleb-johnson merged commit 01b8ef5 into Qiskit:main Jun 12, 2023
@caleb-johnson caleb-johnson deleted the test-build-ci branch June 12, 2023 17:59
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.

3 participants