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: scaffold integration files in app #18763

Merged
merged 2 commits into from
Nov 14, 2021

Conversation

ZachJW34
Copy link
Contributor

@ZachJW34 ZachJW34 commented Nov 2, 2021

Details

This PR moves the scaffolding of integration files into the app using the new code-generation utility. This will enable the Scaffold example specs UI flow.

The scaffolded integration files come from cypress-kitchen-sink which are pulled down and copied into @packages/example. I didn't want to move them into the templates directory since they might fall out of sync.

The codegen differs from the story and component spec generation so I settled on a data-model for generated files that graphql can serve. This data model should work for our scaffolding inside the launchpad that is currently mocked.

Screen.Recording.2021-11-02.at.5.25.40.PM.mov

Draft PR since I have to fix some tests that relied on the specs being generated within @packages/server

  • Have tests been added/updated?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 2, 2021

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Nov 2, 2021



Test summary

9031 1 102 4Flakiness 1


Run details

Project cypress
Status Failed
Commit bd9de7b
Started Nov 12, 2021 10:29 PM
Ended Nov 12, 2021 10:41 PM
Duration 11:49 💡
OS Linux Debian - 10.9
Browser Multiple

View run in Cypress Dashboard ➡️


Failures

cypress/integration/commands/assertions_spec.js Failed
1 ... > logs 'should' when non available chainer

Flakiness

cypress/integration/commands/net_stubbing_spec.ts Flakiness
1 network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@ZachJW34 ZachJW34 marked this pull request as ready for review November 3, 2021 21:22
@ZachJW34 ZachJW34 requested a review from a team as a code owner November 3, 2021 21:22
@ZachJW34 ZachJW34 requested review from jennifer-shehane, JessicaSachs and tgriesser and removed request for a team November 3, 2021 21:22
@@ -336,4 +336,37 @@ export class ProjectActions {
content: newSpec.content,
}
}

async scaffoldIntegration () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think with the addition of this we could probably break this and the other CodeGen related methods out to another Action file that better encomppasses what they were doing. Maybe something like CodeGenActions

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 agree, and I think it could be further improved by moving a lot of stuff out of ProjectDataSource that is related to codegen. I'll tackle that after the create spec flow is in though.

@@ -10,7 +10,7 @@ export interface ProjectShape {
projectRoot: string
}

const GeneratedSpec = objectType({
export const GeneratedSpec = objectType({
Copy link
Contributor

Choose a reason for hiding this comment

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

For this I think we should keep it to one objectType per file. It would be easier to find gql-GeneratedSpec then remember what it's paired with

},
})

export const CodeGenResultWithFileParts = objectType({
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here with multiple objectTypes

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'm going to keep this one in here since I'm really only extending the type.

@ImCesar ImCesar removed the request for review from jennifer-shehane November 3, 2021 21:58
@ZachJW34 ZachJW34 force-pushed the scaffold-integration-in-app branch 2 times, most recently from aad69dc to 3e6e075 Compare November 4, 2021 02:41
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Some comments!

@@ -14,9 +14,12 @@
<Button @click="codeGenTypeClicked('integration')">
Generate Integration
</Button>
<Button @click="codeGenTypeClicked('scaffoldIntegration'); scaffoldIntegration.executeMutation({})">
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create a method on <script> for this, to keep <template> more thin and simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This page was deleted with Jess' recent merge of the create-spec flow, but yeah I should have had moved that into a function.

@@ -847,7 +847,10 @@ export class ProjectBase<TServer extends Server> extends EE {

if (scaffoldExamples) {
debug('will scaffold integration and fixtures folder')
push(scaffold.integration(cfg.integrationFolder, cfg))
if (!process.env.LAUNCPAD) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Uh oh... LAUNCHPAD. This will always be true!

Copy link
Contributor Author

@ZachJW34 ZachJW34 Nov 8, 2021

Choose a reason for hiding this comment

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

This should always be true when running the new launchpad which is what we want. There are some system-tests for the old desktop-gui that still rely on the old scaffolding and this gating was to allow those to continue to work while allowing the new functionality for launchpad. @tgriesser am I correct here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am saying you made a typo - you are missing the N in LAUNCPAD

<RouterLink
:key="generatedSpec.fileParts.absolute"
class="text-left"
:to="{ path: 'runner', query: { file: generatedSpec.fileParts.relative } }"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to immediately execute the spec? I think the route has changed to something like /spec?file=....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This page was deleted, I'll integrate the scaffold integration into the new flow.

@lmiller1990
Copy link
Contributor

Just one comment (re spelling of launchpad). Once this is resolved we can merge

@JessicaSachs JessicaSachs force-pushed the scaffold-integration-in-app branch from 8207e54 to ca88b66 Compare November 10, 2021 01:39
@ZachJW34 ZachJW34 force-pushed the scaffold-integration-in-app branch from ca88b66 to 4169ee0 Compare November 10, 2021 16:26
@ZachJW34 ZachJW34 force-pushed the scaffold-integration-in-app branch from 4169ee0 to 0cf3451 Compare November 10, 2021 17:07
@lmiller1990 lmiller1990 merged commit c66a890 into unified-desktop-gui Nov 14, 2021
@lmiller1990 lmiller1990 deleted the scaffold-integration-in-app branch November 14, 2021 23:44
tgriesser added a commit that referenced this pull request Nov 17, 2021
* 10.0-release:
  feat: improve vite DX (#18937)
  feat: Use plugins on config files (#18798)
  BREAKING CHANGE: trigger major bump
  BREAKING CHANGE: trigger major bump
  fix: fix cypress/package.json crasher
  fix(breaking): change circle.yml to release binary
  fix: build-prod-ui deps before build-prod packages
  feat: implement spec list tree (#18901)
  chore: adding 10.0-release to the circle.yml build script (#18926)
  feat(app): remove __vite__ route and default to unified runner (#18909)
  fix: app layout + specs list review (#18862)
  feat(app): show previous versions (#18838)
  feat: scaffold integration files in app (#18763)
  feat: add footer to the settings (#18867)
  fix: Exit when both --e2e and --component flags are passed in (#18855)
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