feat: IATR-M0 Page Header #24722

amehta265 · 2022-11-17T17:08:43Z

Closes [IATR](M0) Failures page header component #24442

User facing changelog

This component is the header of the new debug page that represents the chosen run and its metadata.
The design for this page can be found here

Additional details

The DebugPageHeader contains metadata related to a user's test. In order to render the status of the test (i.e. number of passed, failed, skipped, pending tests) I have also created another component DebugResults. DebugResults utilizes an already existing component ResultCounts to render the UI for this section. Changes have been ResultCounts.vue to accommodate for a change in the order of statuses shown as per design requirements.
debugPageHeader accepts gql values as a prop. In case these props are not passed to the component it will render default values.
Certain functions such as commitsAhead() are merely placeholders and will be implemented later on. Furthermore the gql query will potentially change in the future

Steps to test

This issue has 3 component tests. Checkout to this branch to run the tests
Both DebugPageHeader.vue and DebugResults.vue exist in the packages/app/ folder. To run these tests:

Change directory to packages/app
Run yarn dev
Navigate to Component Testing and run DebugPageHeader.cy.tsx and DebugResults.cy.tsx

To test ResultsCounts.vue

Go to the cypress root directory
Run yarn cypress:open
Add frontend-shared as a project
Open frontend-shared in Component Testing and run ResultCounts.cy.tsx

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?

merging with feature branch

cypress · 2022-11-17T17:41:39Z

Test summary

436 • 1 • 10 • 0 • 2

Run details


Project	cypress
Status	Failed
Commit	`1271412`
Started	Nov 22, 2022 6:07 AM
Ended	Nov 22, 2022 6:19 AM
Duration	12:29 💡
OS	Linux Debian -
Browser	Chrome 106

View run in Cypress Dashboard ➡️

Failures

	cypress/e2e/runs.cy.ts		1
1	App: Runs > refetching > should fetch newer runs and maintain them when navigating

Flakiness

	runner/reporter-ct-mount-hover.cy.ts	1
1	CT Mount angular-14 > While hovering on Mount(), shows component on AUT for angular-14
	specs_list_latest_runs.cy.ts	1
1	App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs

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

Still looking through details, but wanted to get this feedback to you.

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

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

packages/app/src/debug/DebugResults.vue

packages/app/src/debug/DebugPageHeader.vue

ZachJW34 · 2022-11-17T22:38:36Z

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

-  ]
+  const acc: status[] = []
+
+  if (props.order) {


Should the order even be configurable? The results this component displays should be consistent across features, otherwise it becomes confusing.

This is an attempt to make the ResultCounts.vue component more generic. In its current implementation, the status can only appear in a certain order i.e. skipped, pending, passed, failed. You can see this generic component in the Runs tab for the app.
Based on the design requirements for this issue, the order of these "statuses" is different. Hence, instead of building a whole new component we can utilize ResultCounts.vue and just pass in a different order incase in the future the order of "statuses" is different.

As for the component being consistent, I agree with that. I don't see why the "status" order should differ across pages. However this has been added to meet the design requirements

I would sync back with design regarding this, this result of this list (in my mind) should be consistent regardless of if you're looking at the runs page or the debug page

packages/app/src/debug/DebugResults.vue

packages/app/src/debug/DebugPageHeader.vue

lmiller1990

Sure looks pretty, left some comments

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

packages/app/src/debug/DebugPageHeader.vue

lmiller1990 · 2022-11-18T06:46:32Z

packages/app/src/debug/DebugPageHeader.vue

+
+</script>
+<style scoped>
+ul:nth-child(2) li:not(:first-child)::before {


Woah, this is specific. I think this CSS could break easily.

I'm not sure what it does, but can we use a class or id or something that isn't tied to a specific DOM structure?

This adds the " . " after each component in the bottom section of the UI. That way we don't manually have to add " . " in our template. It is similar to styling here

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

lmiller1990 · 2022-11-18T06:48:26Z

packages/app/src/debug/DebugResults.vue

+        <div
+          class="text-red-400 font-semibold text-sm flex items-center border-r-1px px-2"
+        >
+          <component


Why not just

<FailedSolidIcon class=... data-cy=... />

<component> is good for dynamic content - but this isn't. (Could be missing something).

This will not always be a FailedSolidIcon. In future iterations, this could be a PassedIcon or hold other statuses hence the <component>

lmiller1990 · 2022-11-18T06:49:37Z

packages/app/src/debug/DebugResults.vue

+import type { DebugResultsFragment } from '../generated/graphql'
+import { gql } from '@urql/core'
+import FailedSolidIcon from '~icons/cy/status-failed-solid_x24.svg'
+import ResultCounts from '@packages/frontend-shared/src/components/ResultCounts.vue'


Curious what the team things but this seems like it's not really a good candidate for frontend-shared. It doesn't seem like something we will use in other packages (like launchpad).

Thoughts? cc @marktnoonan, we had a brief discussion around this.

@lmiller1990 The ResultsCounts is an existing component that was just getting reused here with a minor addition. I do agree that it is only used in app so does not need to be in frontend-shared, but we can refactor that another time.

Sounds good to me

warrensplayer

Just one minor nit-pick

packages/app/src/debug/DebugPageHeader.vue

warrensplayer · 2022-11-21T19:36:49Z

packages/app/src/debug/DebugResults.vue

+import type { DebugResultsFragment } from '../generated/graphql'
+import { gql } from '@urql/core'
+import FailedSolidIcon from '~icons/cy/status-failed-solid_x24.svg'
+import ResultCounts from '@packages/frontend-shared/src/components/ResultCounts.vue'


@lmiller1990 The ResultsCounts is an existing component that was just getting reused here with a minor addition. I do agree that it is only used in app so does not need to be in frontend-shared, but we can refactor that another time.

lmiller1990

Looks good, left one last comment but not a blocker.

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

lmiller1990 · 2022-11-21T23:09:20Z

packages/app/src/debug/DebugResults.vue

+import type { DebugResultsFragment } from '../generated/graphql'
+import { gql } from '@urql/core'
+import FailedSolidIcon from '~icons/cy/status-failed-solid_x24.svg'
+import ResultCounts from '@packages/frontend-shared/src/components/ResultCounts.vue'


Sounds good to me

lmiller1990 · 2022-11-21T23:37:45Z

Updating the feature branch and merging it in here, should fix CI. I'll merge once this is green.

lmiller1990 · 2022-11-22T06:23:10Z

Looks like we have a CI failure - re-ran but still failing, want to take a look @amehta265? https://app.circleci.com/pipelines/github/cypress-io/cypress/46041/workflows/a86994bb-0f07-454a-bfaf-60c6048476a0/jobs/1936007/tests#failed-test-0

lmiller1990 · 2022-11-22T13:02:09Z

Actually @amehta265 it might be flake, see #24575
If you want to verify it passes locally, I'd recommending pushing a commit to skip this and reference #24575 so we can fix it next sprint.

…press-io/cypress into ankit/IATR-M0-debugPageHeader Merging with remote branch

Closes undefined

amehta265 added 6 commits November 15, 2022 19:02

wip ui header bar

9488877

upper portion of header complete

21e2ef1

Merge branch 'feature/IATR-M0' into ankit/IATR-M0-debugPageHeader

e777708

merging with feature branch

adding tests for debug page header

377edd1

completed all tests and updated components

e70841e

updating mount for tests

7adc601

amehta265 requested review from a team and warrensplayer November 17, 2022 17:14

warrensplayer requested changes Nov 17, 2022

View reviewed changes

warrensplayer requested a review from a team November 17, 2022 20:46

ZachJW34 reviewed Nov 17, 2022

View reviewed changes

lmiller1990 reviewed Nov 18, 2022

View reviewed changes

amehta265 added 3 commits November 18, 2022 10:55

updating styling and props PR comments

039f878

removed default props and updated debugResults based on PR comments

2582908

updated tests

3aeae99

amehta265 changed the title ~~feature: IATR-M0 Page Header~~ feat: IATR-M0 Page Header Nov 21, 2022

warrensplayer requested review from a team, ZachJW34, lmiller1990 and warrensplayer November 21, 2022 18:32

warrensplayer reviewed Nov 21, 2022

View reviewed changes

generalizing styling in pageHeader

854c17a

warrensplayer approved these changes Nov 21, 2022

View reviewed changes

lmiller1990 approved these changes Nov 21, 2022

View reviewed changes

Merge branch 'feature/IATR-M0' into ankit/IATR-M0-debugPageHeader

1271412

updating tests to use findByTestId

aff23a7

amehta265 added 2 commits November 22, 2022 08:04

Merge branch 'ankit/IATR-M0-debugPageHeader' of https://github.com/cy…

66f12b9

…press-io/cypress into ankit/IATR-M0-debugPageHeader Merging with remote branch

skipping flaky test

2913ec0

amehta265 merged commit 5d44df4 into feature/IATR-M0 Nov 22, 2022

amehta265 deleted the ankit/IATR-M0-debugPageHeader branch November 22, 2022 14:45

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

[IATR](M0) Failures page header component #24442

Closed

warrensplayer mentioned this pull request Dec 8, 2022

feat: IATR-M0 Debug Page #25067

Merged

1 task

warrensplayer pushed a commit that referenced this pull request Jan 6, 2023

feat: IATR-M0 Page Header (#24722) (#25360)

c33ba30

Closes undefined

feat: IATR-M0 Page Header #24722

feat: IATR-M0 Page Header #24722

Uh oh!

Conversation

amehta265 commented Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

Uh oh!

cypress bot commented Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test summary

Run details

Failures

Flakiness

Uh oh!

warrensplayer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lmiller1990 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warrensplayer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiller1990 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiller1990 commented Nov 21, 2022

Uh oh!

lmiller1990 commented Nov 22, 2022

amehta265 commented Nov 17, 2022 •

edited

Loading

cypress bot commented Nov 17, 2022 •

edited

Loading