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

chore: minor style updates & fix a windows test #24558

Merged
merged 25 commits into from Nov 8, 2022

Conversation

marktnoonan
Copy link
Contributor

This PR fixes a windows Launchpad test that was failing due to config being slow to load. It's only possible to test the "record first run" prompt after a testing type has been chosen, which means the test needs to wait for the "Choose a Browser" screen to appear.

Also tweaks the border of the Major Version Welcome page to match the color in figma, and removes an extra overflow utility class that was causing a double scrollbar on Windows.

User facing changelog

Additional details

Steps to test

How has the user experience changed?

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

cypress-bot bot commented Nov 7, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Nov 7, 2022



Test summary

4581 0 343 0Flakiness 3


Run details

Project cypress
Status Passed
Commit aa5c5fa
Started Nov 8, 2022 7:50 AM
Ended Nov 8, 2022 8:02 AM
Duration 12:05 💡
OS Linux Debian -
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

specs_list_latest_runs.cy.ts Flakiness
1 App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs
cypress-in-cypress.cy.ts Flakiness
1 Cypress in Cypress > resets selector playground validity when selecting element with playground selector in component
runner/reporter.hooks.cy.ts Flakiness
1 hooks > only displays tests with .only

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
Copy link
Member

Double scroll bar is gone. It says "11.0.0 Released next month", but users will see it this month. Do we even need a byline? How about just "11.0.0 - latest release"?

image

@lmiller1990 lmiller1990 self-requested a review November 7, 2022 06:08
Copy link
Member

@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.

Fix works, left comment about other wording.

@warrensplayer warrensplayer self-requested a review November 7, 2022 13:24
@warrensplayer
Copy link
Contributor

warrensplayer commented Nov 7, 2022

Double scroll bar is gone. It says "11.0.0 Released next month", but users will see it this month. Do we even need a byline? How about just "11.0.0 - latest release"?

image

I fixed the values being passed to Date.UTC that was a parameter to useTimeAgo. The monthIndex parameter is 0 based. The display will be a little strange during the day tomorrow because it will report that the release was done at midnight UTC, but then will be correct as time progresses.

After fix:
image

@marktnoonan
Copy link
Contributor Author

Good catch @warrensplayer! We need to update the test for "11.0.0 Released just now"

// First ensure the test is loaded
cy.get('.passed > .num').should('contain', '--')
Copy link
Member

Choose a reason for hiding this comment

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

Race condition - sometimes the rest actually executes fast enough, and it's not --, but the number of passes. So, better just verify the elements exist.

@@ -10,8 +10,6 @@
"vue": "^2.6.11"
},
"devDependencies": {
"@cypress/vue2": "file:../../../npm/vue2",
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't do what you expect on windows apparently. Also, seems like it isn't strictly required.

@lmiller1990 lmiller1990 self-requested a review November 8, 2022 07:52
Copy link
Member

@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.

I messed around a bit to get windows to a reasonable state. There is some flake I could not solve, but it's windows only and the behaviors are working fine and verified via linux tests; I skipped them and created issues in the interest of getting this merged.

Looks like there is still some flake, but far less than before.

I did skip ci for the last commit. I checked CI before merging; everything is .

@lmiller1990 lmiller1990 merged commit 5e3a21c into develop Nov 8, 2022
3 checks passed
@lmiller1990 lmiller1990 deleted the marktnoonan/windows-test-updates branch November 8, 2022 08:18
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

5 participants