feat: run failures enhancements #25234

ZachJW34 · 2022-12-20T17:39:58Z

Closes [IATR](M1.0) "Run Failures" button enhancements #24854

User facing changelog

na

Additional details

Working with Ankit revealed we needed to change up DebugMappings.ts. We paired on this and the results of the refactor are relevant for his upcoming work (groups).

Steps to test

Driven mostly from CT tests, if you want to run it e2e:

Check out the feature/CYCLOUD-665-IATR and run yarn
Run yarn docker-compose && yarn dev
Visit localhost:3000 and create a project
In the Cypress monorepo, checkout this branch and run CYPRESS_INTERNAL_ENV=development yarn cypress:open
Select a project and add your newly created projectId
Tweak tests to cause a failure and run npx cypress run --record --key <RECORD_KEY>
If you've recorded different runs, you can change the run by changing runByNumber(runNumber: 6) { to whatever run you want to target (not dynamic yet)

How has the user experience changed?

Screen.Recording.2022-12-20.at.11.31.21.AM.mov

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 · 2022-12-20T17:40:02Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

ZachJW34 · 2022-12-20T17:41:40Z

packages/frontend-shared/src/components/Button.vue

-const variantClasses = computed(() => (VariantClassesTable[props.variant || 'primary']))
+const variantClasses = computed(() => {
+  return (VariantClassesTable[props.variant || 'primary']).split(' ').filter((css) => {
+    return css !== 'hocus-default' || !props.disabled


A button should not have hover states if it is disabled.

I see how this solves the issue for the button type being used in this issue, but it is not really clear here what it is doing or why. Your comment here in the PR was helpful.

Can you at least separate out the additional functionality and not chain it off the original part so that you can add a good comment to explain why this is being performed?

ZachJW34 · 2022-12-20T17:42:14Z

packages/frontend-shared/src/components/Tooltip.vue

@@ -71,6 +71,17 @@ const actualPopperClass = computed(() => {
    result.push('no-arrow')
  }

+  // color takes priority, else fallback to tooltip -> dark, menu -> light


Refactored so as to allow menu functionality styled with the dark tooltip theme (tooltips don't stay open when the mouse leaves the designated hover element, but the tooltip added in this PR has a call to action).

cypress · 2022-12-20T18:10:32Z

Test summary

26243 • 1 • 1378 • 0 • 24

Run details


Project	cypress
Status	Failed
Commit	`3e21acc`
Started	Dec 22, 2022 3:10 PM
Ended	Dec 22, 2022 3:28 PM
Duration	17:58 💡
OS	Linux Debian -
Browser	Multiple

View run in Cypress Dashboard ➡️

Failures

	cypress/e2e/e2e/origin/commands/waiting.cy.ts		1
1	cy.origin waiting > alias > waits for the route alias to have a response

Flakiness

	create-from-component.cy.ts	1
1	... > runs generated spec
	commands/net_stubbing.cy.ts	4
1	... > with `resourceType` > can match a proxied image request by resourceType
2	network stubbing > intercepting response > can delay a proxy response using res.setDelay
3	network stubbing > intercepting response > can throttle a proxy response using res.setThrottle
4	... > stops waiting when an xhr request is canceled
This comment includes only the first 5 flaky tests. See all 24 flaky tests in the Cypress Dashboard.

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

warrensplayer

Overall is great! I love the addition of the types in the DebugMapping.ts file. Makes things much easier to understand. Mostly nit-pick stuff in my comments.

warrensplayer · 2022-12-20T22:32:22Z

packages/frontend-shared/src/components/Button.vue

-const variantClasses = computed(() => (VariantClassesTable[props.variant || 'primary']))
+const variantClasses = computed(() => {
+  return (VariantClassesTable[props.variant || 'primary']).split(' ').filter((css) => {
+    return css !== 'hocus-default' || !props.disabled


I see how this solves the issue for the button type being used in this issue, but it is not really clear here what it is doing or why. Your comment here in the PR was helpful.

Can you at least separate out the additional functionality and not chain it off the original part so that you can add a good comment to explain why this is being performed?

packages/app/src/debug/DebugContainer.vue

packages/app/src/debug/utils/DebugMapping.ts

packages/frontend-shared/src/components/Button.vue

warrensplayer · 2022-12-21T16:08:19Z

packages/frontend-shared/src/components/Tooltip.vue

@@ -46,7 +46,7 @@ const props = withDefaults(defineProps<{
  instantMove?: boolean
  showGroup?: string
 }>(), {
-  color: 'dark',
+  color: undefined,


What did the component do with the color prop originally. It does not look like it was used. I am guessing it just got passed on to the Menu or Tooltip components from floating-vue. I looked at the documentation for that library but could not find any documentation for it. Did you look?

Couldn't find any docs for it and it wasn't being referenced so I hijacked it!

packages/frontend-shared/src/components/Tooltip.vue

astone123 · 2022-12-21T21:19:49Z

Is the tooltip supposed to look like this? Looks like the body needs to get moved up a couple of pixels

astone123 · 2022-12-21T21:22:28Z

packages/app/src/debug/utils/DebugMapping.ts

      const foundSpec = specs.find((spec) => spec.id === curr.specId)

-      spec = { spec: foundSpec }
-      // console.log('looking for spec', spec)


Remove this comment?

I removed this in a previous commit, maybe the PR was stale when you were reviewing?

astone123 · 2022-12-21T21:22:50Z

packages/frontend-shared/src/components/Tooltip.cy.tsx

+    cy.mount(() => {
+      return (
+        <div class="flex m-100px p-4 w-150px gap-160px">
+          {/* @ts-ignore */}


What is this ts-ignore for?

No idea it was a copy and paste. Let me see if I can remove it without check-ts being pissed.

astone123

Looks good for the most part - just had a couple of comments. I also left a comment on one of the Percy snapshots that changed. It looks like that might actually be related to a change that was made here. We should check that out just to make sure 👍🏻

warrensplayer

The code looks good. The only thing I saw that was strange in the UI was the little arrow on the tooltip. It is a little bit too high above the body of the tooltip. If you zoom in, you can see the bottom arrows of the triangle.

ZachJW34 · 2022-12-21T22:51:59Z

@astone123 I reverted the changes I made to the Button component but the test/snapshot didn't change. Not sure why, I'll check this out on the feature branch.

lmiller1990 · 2022-12-22T04:26:12Z

packages/app/src/debug/DebugContainer.cy.tsx


-      const debugMappingArray = specsList(spec, tests)
+      const debugMappingArray = specsList({ specs, tests, groups, localSpecs: [], currentTestingType: 'e2e' })


Now we are on Vite 4 you should have an easier time adding vitest to the code base. Not needed for this PR to merge, but I'd like to start using Vitest more for non UI tests - as much as I like dogfooding Cypress, it really doesn't feel like the right tool for this class of test.

I know we are on the same page about this, just noting it here since we couldn't add Vitest before, but now we should be able to. Feel free to add next time you see a chance! It's fast ⚡

ZachJW34 · 2022-12-22T15:01:13Z

@astone123 looks like that button was relying on css class ordering which is not the proper way to apply overrides (there isn't an ideal way to apply overrides but bg-gray-500 bg-indigo-500 shouldn't be what we rely on). I've reverted the color change by applying a custom class, though I feel like the disabled primary looked better 😅

feat: run failures enhancements

ba9c65a

remove stub

8ec91a1

ZachJW34 commented Dec 20, 2022

View reviewed changes

ZachJW34 requested review from warrensplayer and astone123 December 20, 2022 17:44

ZachJW34 added 2 commits December 20, 2022 14:12

update cloud stubbed types

30438db

make button filter logic more clear

2cd47ff

warrensplayer reviewed Dec 21, 2022

View reviewed changes

ZachJW34 added 2 commits December 21, 2022 11:42

PR feedback

52feb4b

fix ct test

0c65bf4

astone123 reviewed Dec 21, 2022

View reviewed changes

astone123 approved these changes Dec 21, 2022

View reviewed changes

warrensplayer reviewed Dec 21, 2022

View reviewed changes

feedback

f40793f

lmiller1990 approved these changes Dec 22, 2022

View reviewed changes

override button css

3e21acc

warrensplayer approved these changes Dec 22, 2022

View reviewed changes

ZachJW34 merged commit 216d4c5 into feature/IATR-M0 Dec 22, 2022

ZachJW34 deleted the zachw/run-failures-enh branch December 22, 2022 15:56

cypress-bot bot mentioned this pull request Dec 22, 2022

[IATR](M1.0) "Run Failures" button enhancements #24854

Closed

warrensplayer mentioned this pull request Jan 5, 2023

[IATR] E2E Tests for IATR #25351

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: run failures enhancements #25234

feat: run failures enhancements #25234

ZachJW34 commented Dec 20, 2022

cypress-bot bot commented Dec 20, 2022

ZachJW34 Dec 20, 2022

warrensplayer Dec 20, 2022

ZachJW34 Dec 20, 2022 •

edited

Loading

cypress bot commented Dec 20, 2022 •

edited

Loading

warrensplayer left a comment

warrensplayer Dec 20, 2022

warrensplayer Dec 21, 2022

ZachJW34 Dec 21, 2022

astone123 commented Dec 21, 2022

astone123 Dec 21, 2022

ZachJW34 Dec 21, 2022

astone123 Dec 21, 2022

ZachJW34 Dec 21, 2022

astone123 left a comment

warrensplayer left a comment

ZachJW34 commented Dec 21, 2022

lmiller1990 Dec 22, 2022 •

edited

Loading

ZachJW34 commented Dec 22, 2022


		const debugMappingArray = specsList(spec, tests)
		const debugMappingArray = specsList({ specs, tests, groups, localSpecs: [], currentTestingType: 'e2e' })

feat: run failures enhancements #25234

feat: run failures enhancements #25234

Conversation

ZachJW34 commented Dec 20, 2022

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Dec 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZachJW34 Dec 20, 2022 • edited Loading

Choose a reason for hiding this comment

cypress bot commented Dec 20, 2022 • edited Loading

Test summary

Run details

Failures

Flakiness

warrensplayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astone123 commented Dec 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astone123 left a comment

Choose a reason for hiding this comment

warrensplayer left a comment

Choose a reason for hiding this comment

ZachJW34 commented Dec 21, 2022

lmiller1990 Dec 22, 2022 • edited Loading

Choose a reason for hiding this comment

ZachJW34 commented Dec 22, 2022

ZachJW34 Dec 20, 2022 •

edited

Loading

cypress bot commented Dec 20, 2022 •

edited

Loading

lmiller1990 Dec 22, 2022 •

edited

Loading