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

feat(app): persist current spec in data context, update runner workflow for demo #18406

Merged
merged 11 commits into from
Oct 11, 2021

Conversation

lmiller1990
Copy link
Contributor

@lmiller1990 lmiller1990 commented Oct 8, 2021

Did:

  • added currentSpec field to activeProject
  • persist selected spec to data context
  • when selecting a spec, redirect to /runner route and run spec
  • NOTE: only working for CT. E2E is WIP.

I am not sure why but the app component tests are flaky - seems like a Vite issue. The launchpad CT tests seem fine, though. You can test it like this:

  1. cd packages/launchpad
  2. yarn dev
  3. Choose CT
  4. Navigate to /__vite__/ in the Cypress browser
  5. Choose a spec

Known issues (to solve in other PRs, to avoid this getting huge):

  • At this point we unmount the AUT and Reporter, and remount them for each spec. That's not the most ideal workflow, but it at least unblocks working on the UI, and will give us something to demo.
  • Some driver commands are not hooked up yet with the AUT - you can see in the demo, cy.viewport fails. This is going to be fixed as part of the iframes ticket
  • There are also some errors in the console - these will be resolved in the future. Some kind of race condition, that was previous not possible?

Demo!

demook.mov

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 8, 2021

Thanks for taking the time to open a PR!

@lmiller1990 lmiller1990 marked this pull request as ready for review October 8, 2021 07:57
@lmiller1990 lmiller1990 requested a review from a team as a code owner October 8, 2021 07:57
@lmiller1990 lmiller1990 requested review from chrisbreiding and BlueWinds and removed request for a team, chrisbreiding and BlueWinds October 8, 2021 07:57
@@ -9,32 +9,41 @@
<div>{{ t('specPage.componentSpecsHeader') }}</div>
<div>{{ t('specPage.gitStatusHeader') }}</div>
</div>
<router-link
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now just using gql as the source of truth, not sure if there's a specific reason we need to keep the current spec in the URL or not 🤔

// TODO: Do we need this many ways to describe a spec?

// represents a spec file on file system and
export interface BaseSpec {
Copy link
Contributor Author

@lmiller1990 lmiller1990 Oct 8, 2021

Choose a reason for hiding this comment

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

Many parts of the system (mainly server) assume a "spec" is exactly

absolute:
relative:
name:

and nothing else. Our definitions of a "spec" are getting kinda messy, we need to unify these eventually.

@@ -426,6 +426,11 @@ export const eventManager = {
})

Cypress.on('log:added', (log) => {
// TODO: Race condition in unified runner - we should not need this null check
Copy link
Member

Choose a reason for hiding this comment

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

Do we know anything about the cause of this race condition / any potential impact?

Copy link
Contributor Author

@lmiller1990 lmiller1990 Oct 8, 2021

Choose a reason for hiding this comment

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

I looked and it seems like Cypress is trying to log GraphQL requests as the AUT is unmounted (eg, if you click to return to the Spec page, it makes a GraphQL request).

I think I need to ensure the driver has fully stopped and been torn down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will be pairing with Brian this week on this.

@tgriesser
Copy link
Member

I am not sure why but the app component tests are flaky - seems like a Vite issue. The launchpad CT tests seem fine, though. You can test it like this:

@lmiller1990 The tests aren't flaky, they weren't passing in #18372 due to the change in the history routing history: createWebHistory('/__vite__/'),. Fixed it in #18414

We need to hold off merging until the required status checks are passing given then number of folks that are now working off of this branch across teams.

@lmiller1990 lmiller1990 merged commit b07ccf8 into unified-desktop-gui Oct 11, 2021
tgriesser added a commit that referenced this pull request Oct 11, 2021
* unified-desktop-gui:
  feat(app): persist current spec in data context, update runner workflow for demo (#18406)
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