Skip to content

fix: windows#22234

Merged
ryanthemanuel merged 12 commits intodevelopfrom
ryanm/playground/fix-windows-build
Jun 10, 2022
Merged

fix: windows#22234
ryanthemanuel merged 12 commits intodevelopfrom
ryanm/playground/fix-windows-build

Conversation

@ryanthemanuel
Copy link
Copy Markdown
Collaborator

@ryanthemanuel ryanthemanuel commented Jun 9, 2022 •
edited
Loading

User facing changelog

N/A

Additional details

The main goal here is to fix flaky tests in windows. This was accomplished via:

  • Bump the resource class of the windows executor
  • Add some waits to a few places that seem to rely on postFragment subscriptions

Neither of these are ideal solutions; however, they don't appear to be new problems (I can recreate them in at least v10.0.3). Consequently we want to make the builds more stable and follow up on these issues later.

Steps to test

Check that CI succeeds

How has the user experience changed?

N/A

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Copy Markdown
Contributor

cypress-bot Bot commented Jun 9, 2022

Thanks for taking the time to open a PR!

@ryanthemanuel ryanthemanuel changed the title fix windows fix: windows Jun 9, 2022
@cypress
Copy link
Copy Markdown

cypress Bot commented Jun 9, 2022 •
edited
Loading


Test summary

37553 • 0 • 454 • 0 • Flakiness 4

Run details
Project cypress
Status Passed
Commit 94f46e4
Started Jun 10, 2022 1:04 PM
Ended Jun 10, 2022 1:20 PM
Duration 16:33 💡
OS Linux Debian - 10.11
Browser Multiple

View run in Cypress Dashboard ➡️

Flakiness

e2e/origin/commands/actions.cy.ts 2 Flakiness
1 cy.origin actions > #consoleProps > .selectFile()
2 cy.origin actions > #consoleProps > .submit()
cypress/proxy-logging.cy.ts 1 Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
e2e/origin/commands/assertions.cy.ts 1 Flakiness
1 cy.origin assertions > #consoleProps > .should() and .and()

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

lmiller1990
lmiller1990 approved these changes Jun 10, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Looks good, I will find out why #22118 has (frustrating) windows on CI fails. I'm fairly sure they are just flake, they've passed before, but I'll find out.

Comment thread circle.yml
name: windows-create-build-artifacts
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
Copy link
Copy Markdown
Contributor

@lmiller1990 lmiller1990 Jun 10, 2022

Choose a reason for hiding this comment

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

A little unsettling that we don't know why this is required, but glad to have windows CI back to green.

Copy link
Copy Markdown
Contributor

@mjhenkes mjhenkes Jun 10, 2022

Choose a reason for hiding this comment

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

Agreed. 😕

Comment thread packages/data-context/src/actions/MigrationActions.ts
@@ -32,9 +32,6 @@ import {
} from '../sources/migration'
Copy link
Copy Markdown
Contributor

@lmiller1990 lmiller1990 Jun 10, 2022

Choose a reason for hiding this comment

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

Just to clarify for reviewers: every file change below this line is reverted from #22118 and you don't need to review it to review this PR.

This was referenced Jun 10, 2022
Merged
Merged
@lmiller1990
Copy link
Copy Markdown
Contributor

lmiller1990 commented Jun 10, 2022 •
edited
Loading

Hey @ryanthemanuel I do not think we need to revert #22118 anymore - can you just merge this commit and it'll be fine. I checked out #22118, merged this in, added that one commit, and pushed - CI is green. Link to CI run with 100% windows tests passing first run.

@ryanthemanuel ryanthemanuel marked this pull request as ready for review June 10, 2022 13:55
@ryanthemanuel ryanthemanuel requested a review from tgriesser as a code owner June 10, 2022 13:55
@ryanthemanuel ryanthemanuel merged commit 600daef into develop Jun 10, 2022
@ryanthemanuel ryanthemanuel deleted the ryanm/playground/fix-windows-build branch June 10, 2022 14:04
This was referenced Jun 11, 2022
Merged
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgriesser tgriesser Awaiting requested review from tgriesser

2 more reviewers

@mjhenkes mjhenkes mjhenkes approved these changes

@lmiller1990 lmiller1990 lmiller1990 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ryanthemanuel @lmiller1990 @mjhenkes