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 tutorial provider #645

Merged
merged 11 commits into from
Sep 8, 2023
Merged

Add tutorial provider #645

merged 11 commits into from
Sep 8, 2023

Conversation

kissiel
Copy link
Contributor

@kissiel kissiel commented Jul 27, 2023

Description

This PR introduces a Checkbox Tutorial Provider.
The included test plan contains various jobs that yield different outcomes, including outcomes that are results of failing resources or dependent jobs.

Tested throughout manually 👨‍🔧

Resolves: CHECKBOX-750

Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

One question inline, and also one request: as I pointed out in Jira, I need a job that uses an environment variable (with a default set), because of this section in the tutorial.

So basically a job that runs something like

echo ${TUTO:-default}

so that it still outputs something if no env var is set, but behaves differently if an env var is set (in launcher, in my case).

providers/tutorial/manage.py Outdated Show resolved Hide resolved
providers/tutorial/units/jobs.pxu Show resolved Hide resolved
@pieqq
Copy link
Collaborator

pieqq commented Aug 11, 2023

Another request for this PR: the tutorial needs to be available from an installed snap, so the snapcraft.yaml recipes need to package it, the same way we package provider-sru for instance.

@pieqq pieqq marked this pull request as draft August 30, 2023 09:42
@pieqq pieqq requested a review from Hook25 August 30, 2023 13:45
@pieqq pieqq marked this pull request as ready for review August 30, 2023 13:46
@pieqq pieqq self-requested a review August 30, 2023 13:46
@pieqq
Copy link
Collaborator

pieqq commented Aug 30, 2023

I've made the modifications I talked about in my review, and a few other things.

I've also added this provider to the checkbox22 snap. Not sure if it's useful to add it to others. Thoughts on that?

Finally, regarding the jobs, I'm wondering if we should drop the description fields and replace them with summary, since the latter will be used in the UI, but not the former.

@pieqq pieqq requested review from tang-mm and pieqq and removed request for pieqq August 31, 2023 02:59
@pieqq pieqq marked this pull request as draft September 5, 2023 14:20
@pieqq
Copy link
Collaborator

pieqq commented Sep 5, 2023

Revisiting this:

  • yes, description should be replaced by summary, otherwise user will see job ids in the test selection screen
  • a manual job is needed to show the user how to proceed with them
  • probably also need a user-interact and a user-interact-verify job as well

@pieqq
Copy link
Collaborator

pieqq commented Sep 6, 2023

I've done the following:

  • Add 3 interactive jobs (manual, user-interact and user-interact-verify) to demonstrate them when running the Tutorial test plan
  • Add a Tutorial category so that our Tutorial jobs are not in the Uncategorised section of the test plan (it looked lame in the Tutorial page)
  • Add some output to the passing and failing job for demonstration purposes (see Add "Running your first test plan" tutorial #705)
  • Change description to summary so that we see a nice list of jobs summaries in the job selection screen

@pieqq pieqq marked this pull request as ready for review September 6, 2023 06:16
@pieqq pieqq requested a review from yphus September 6, 2023 06:17
Copy link
Collaborator

@tang-mm tang-mm left a comment

Choose a reason for hiding this comment

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

Changes in docs/ look good to me

Copy link
Contributor Author

@kissiel kissiel left a comment

Choose a reason for hiding this comment

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

Thanks for picking up the branch!
One question below

Copy link
Contributor Author

@kissiel kissiel left a comment

Choose a reason for hiding this comment

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

I can't officially +1 this PR as I opened it, but big +1 from me!

Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

Yay, I have to approve my own changes! Thank you GitHub!

@pieqq pieqq merged commit d47079a into main Sep 8, 2023
15 checks passed
@pieqq pieqq deleted the add-tutorial-provider branch September 8, 2023 08:08
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.

None yet

3 participants