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

Multi-Session Tests and Tutorial #709

Merged
merged 25 commits into from
Apr 3, 2024
Merged

Multi-Session Tests and Tutorial #709

merged 25 commits into from
Apr 3, 2024

Conversation

garrettmflynn
Copy link
Member

This PR extends #708 to run multi-session tests and use the generated screenshots in a Multi-Session Tutorial in the Read the Docs.

@garrettmflynn garrettmflynn self-assigned this Mar 28, 2024
@garrettmflynn garrettmflynn changed the base branch from main to singlesession-tutorial March 28, 2024 18:42
Base automatically changed from singlesession-tutorial to main March 28, 2024 22:01
@CodyCBakerPhD CodyCBakerPhD marked this pull request as ready for review March 28, 2024 22:14
@CodyCBakerPhD
Copy link
Collaborator

Noticed this changelog includes 'new' figures for the single session case, and that was something I wondered about

I imagine that is simply triggered via locally running the e2e tests (with screenshot capture enabled? Is that something that can be disabled?) and adding+committing resulting 'new' screenshot files?

I do appreciate that GitHub now has sensible image comparison previews, and the files themselves aren't even remotely large, so I'm generally OK with it, just wondering

@garrettmflynn
Copy link
Member Author

Noticed this changelog includes 'new' figures for the single session case, and that was something I wondered about

I imagine that is simply triggered via locally running the e2e tests (with screenshot capture enabled? Is that something that can be disabled?) and adding+committing resulting 'new' screenshot files?

I do appreciate that GitHub now has sensible image comparison previews, and the files themselves aren't even remotely large, so I'm generally OK with it, just wondering

Yeah those updates are all simply a result of re-running the entire test suite—as I am deleting the entire assets/tutorials folder at the start.

I can set a couple of flags to determine how many of the images to delete + regenerate, so we aren't dealing with a whole slew of unrelated changes for each tutorial update.

@garrettmflynn
Copy link
Member Author

Otherwise I just manually selected the screenshots that I expected to change and/or had relevant updates using the GitHub Desktop commit manager. Maybe this is the way to do this with the most flexibility?

@CodyCBakerPhD
Copy link
Collaborator

As long as it doesn't happen automatically on runs, or we're cognisant to stash them instead of committing each time, this should work fine

The hope is that each of those snapshots has a direct analog in the storybook, and that any time a PR induces changes that must be accepted via Chromatic, we'd also bump the snapshots for the tutorial

@CodyCBakerPhD
Copy link
Collaborator

Just a few last minute touchups then this is good to go! Looking really good

@CodyCBakerPhD
Copy link
Collaborator

@garrettmflynn Dang, merging #712 first seems to have caused a number of failures here

@garrettmflynn garrettmflynn changed the base branch from main to ensure-workflow-persistence April 3, 2024 16:14
Base automatically changed from ensure-workflow-persistence to main April 3, 2024 17:52
@CodyCBakerPhD CodyCBakerPhD enabled auto-merge April 3, 2024 19:34
@CodyCBakerPhD CodyCBakerPhD merged commit f67d9c6 into main Apr 3, 2024
14 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the multisession-tutorial branch April 3, 2024 19:46
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.

2 participants