feat: Debug run navigation #26017

lmiller1990 · 2023-03-06T01:29:45Z

Closes Debug Run Navigation #25899
Closes Debug Run Navigation: Frontend Updates #26018

Additional details

There are two main parts to this PR:

addition of the new DebugRunNavigation component and its child DebugRunNavigationRow component
Updates to the RelevantRunsDataSource and RelevantRunSpecsDataSource to support it

The other changes all involve refactoring various parts of the code for reuse, and some general UI fixes/improvements that were found will testing this feature.

Steps to test

You can see various states of this component by opening the App for existing projects (like our own Cypress project), but the easiest way to test will be to have your own Cloud project that you can easily record local runs against.

Scenarios to test:

Start with a project with no runs. When connected to the cloud you should see the existing empty state to record a run
Record the first run in the project, the Debug page should update to show the running build and no run navigation should be present since it is the only run
Make a commit in your project and then record a new run
The debug page should update to show the run navigation component that indicates that the new build is running
- The component will not show the expandable component since there is only one run to switch to
- Click the "Switch to the newest run" button to view the running build
- The component will update to show you are on the most recent build and you should be able to expand the component and switch back to the previous running build

When the running build completes, the component will be removed since there is only one build for the most recent commit with a completed build and no newer builds
Record another run without making a new commit
- The run navigation component will appear again and show the running build
- You can switch back and forth between the runs
When the running build completes, the navigation component will remain. This is because there are multiple builds for the same most relevant commit with builds.

How has the user experience changed?

The Debug page will now show a new component at the top when scenarios allow for changing between different runs for debugging.

See the linked issue #25899 for screenshots and details.

PR Tasks

Have tests been added/updated?
[] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?

cypress · 2023-03-06T01:59:36Z

Passing run #44988 ↗︎

0	580	0	0	0

Details:

Update cli/CHANGELOG.md
Project: cypress	Commit: `66edf0276d`
Status: Passed	Duration: 07:27 💡
Started: Mar 27, 2023 10:33 PM	Ended: Mar 27, 2023 10:40 PM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

warrensplayer

@lmiller1990 I wanted to get you this feedback. When a new build is running, I am getting GQL errors and still looking into what is causing that.

warrensplayer · 2023-03-12T20:23:17Z

packages/app/src/debug/DebugRunDetailedView.vue

+          <li
+            v-for="run of groupByCommit[sha]"
+            :key="run?.runNumber!"
+            class="flex ml-6px mr-12px p-10px pl-30px hocus:bg-indigo-50 cursor-pointer rounded relative ring-3 ring-white ring-inset"


What is the need for the "ring" styles here? It causes the dashed line to get broken up.

I added this to get this gap:

Definitely room to improve the CSS.

warrensplayer · 2023-03-12T20:43:00Z

packages/app/src/debug/DebugRunDetailedView.vue

+      class="relative my-8px"
+      data-cy="debug-historical-runs"
+    >
+      <div class="w-5px left-[15px] absolute border-dashed border-l-0 border-y-0 border-2 border-r-gray-100 h-full" />


Instead of adding a DIV here for the dashed line which is not semantically correct inside a ul, you can add these same classes to a before pseudo element on the ul above.

It would be something like:

before:(content-DEFAULT h-full w-5px border-2 border-dashed border-l-0 border-y-0 border-r-gray-100 left-[15px] absolute)

Also, one option to have it match the design more closely would be to remove the h-full and use a top and bottom offset instead.

before:(content-DEFAULT top-20px bottom-10px w-5px border-2 border-dashed border-l-0 border-y-0 border-r-gray-100 left-[15px] absolute)

warrensplayer · 2023-03-12T20:47:23Z

packages/app/src/debug/DebugRunDetailedView.vue

+const current = computed(() => cloudProject.value?.current)
+
+const latestIsCurrentlySelected = computed(() => {
+  return latest.value?.commitInfo?.sha === current.value?.commitInfo?.sha


This should be comparing run numbers, not shas here.

Suggested change

return latest.value?.commitInfo?.sha === current.value?.commitInfo?.sha

return latest.value?.runNumber === current.value?.runNumber

packages/app/src/debug/DebugRunDetailedView.vue

…9_debug_navigation

lmiller1990

Still testing and going over the code, seems mostly working as expected so far, left some comments/questions.

lmiller1990 · 2023-03-27T07:08:45Z

packages/data-context/src/sources/RelevantRunsDataSource.ts

@@ -1,11 +1,12 @@
 import { gql } from '@urql/core'


Note for reviewers

Above file packages/data-context/src/sources/RelevantRunSpecsDataSource.ts is "Large diffs are not rendered by default" but it's a really critical part of this PR - make sure you expand it and take a look!

lmiller1990 · 2023-03-27T07:13:39Z

packages/data-context/src/sources/RelevantRunSpecsDataSource.ts

    }

-    return SPECS_EMPTY_RETURN
+    const filter = (run: PartialCloudRunWithId) => {


When is run.id not going to be equal to runId? It looks like this is what causes

function subscribed (value: any) { //optional filter for stream of data if (opts?.filter && !opts.filter(value)) { return } // We can get events here before next() is called setting up the deferred promise

To return early - but I think I'm still missing what this does - I'm fairly sure it's to unsubscribe the subscription when the run is no longer running, but I am still figuring out the entirely flow.

I have added some JSDocs to the DataEmitterActions.subscribeTo method to better describe the options. This filter is a predicate to allow for filtering the values being "emitted" for a subscription. Since there can be multiple "subscriptions" tied to the same "event", I needed a way to only send certain values back on certain subscriptions.

In the Run Navigation use case, there could be multiple running builds in the Cloud at one time that the component wants to subscribe to. The backend query will combine them together and execute one query to the Cloud using the cloudNodesById field. When the result comes back, the code iterates over each CloudRun returned to check the cache to see if it changes. If it has changed and an event is sent to the emitter, the filter will only send through the runs with the matching id to the correct upstream (i.e. front end) subscription.

lmiller1990 · 2023-03-27T07:18:13Z

packages/data-context/src/sources/RelevantRunSpecsDataSource.ts

-    if (!this.#poller) {
-      this.#poller = new Poller(this.ctx, 'relevantRunSpecChange', this.#pollingInterval, async () => {
-        const runs = this.ctx.relevantRuns.runs
+  createQuery (infos: GraphQLResolveInfo[]) {


This is pretty clean - it looks like unless someone has to change something significant, this code should be set and forget - it won't really need to be revisited, which is great!

I'm still not 100% sold on dynamically generating the query, since we lose the declarative nature of GraphQL, which is really nice (you can just copy+paste the code into the Graphical interface and run the query, no longer possible here) but I do think this is the best option we have right now.

lmiller1990 · 2023-03-27T07:18:46Z

packages/data-context/src/sources/RelevantRunSpecsDataSource.ts

+
+  const selections = newFieldNode.selectionSet?.selections!
+
+  return {


Nice job figuring this out 💯

lmiller1990 · 2023-03-27T08:22:54Z

packages/data-context/test/unit/sources/fixtures/graphqlFixtures.ts

+        { runNumber: 5, status: 'RUNNING', commitInfo: { sha: FAKE_SHAS[2] } },
+        { runNumber: 4, status: 'FAILED', commitInfo: { sha: FAKE_SHAS[1] } },
+        { runNumber: 1, status: 'PASSED', commitInfo: { sha: FAKE_SHAS[0] } },
+      ],
    },
  },
 }


Not reltaed to this file but I found out that

When I am on the spec page this will show the old status until the new run is completed done - eg, if my run takes 10 min, even if the first test fails, I won't know until the entire run finishes. Is that correct, or should this start auto upadating? Potential bug in polling logic for the Sidebar subscription?

The updates on the debug page are working as expected.

Not reltaed to this file but I found out that

When I am on the spec page this will show the old status until the new run is completed done - eg, if my run takes 10 min, even if the first test fails, I won't know until the entire run finishes. Is that correct, or should this start auto upadating? Potential bug in polling logic for the Sidebar subscription?

The updates on the debug page are working as expected.

This is the expected behavior at this time. There is another initiative called In-App Run Notifications to address providing better feedback to the user about Cloud runs when you are not on the Debug page or not looking at the Cypress app at all.

marktnoonan · 2023-03-27T17:30:36Z

mostly works great, found one small bug with the time incrementing incorrectly after runs complete in some situations: https://www.loom.com/share/5cd609f2ff59433e8c9b8795cc1ba957?highlightComment=11609224&t=435

That video is really long so I'll gather up all the feedback that was actually useful and add it here, mostly around a11y, and a little bit around the legibility of the UI, nothing blocking apart from the bug.

marktnoonan · 2023-03-27T17:53:08Z

packages/app/src/debug/DebugRunNavigationRow.vue

+  >
+    <component
+      :is="isCurrentRun ? 'div': 'button'"
+      :aria-label="t('debugPage.switchToRun', {runNumber: gql.runNumber})"


This aria label has some problems. I think there needs to be a computed for the runNumber to populate, as it's probably just evaluated once on render, meaning we have an empty run number in the label.

But when this renders a button, the presence of aria-label prevents the internal content from being exposed to screen readers, the label becomes the accessible name. When it's a div and not interactive this doesn't seem to be a problem. Loom.

A click hander on a div is fine if there is a button inside that can emit a click and be the focus target for keyboard users, even if the visual style makes the whole row look selected.

I follow all of your feedback. I will create a follow up issue for this. I can see where we also make use of the HeadlessUI radio group component to get better keyboard navigation in that page you mention in the loom.

warrensplayer · 2023-03-27T19:58:03Z

found one small bug with the time incrementing incorrectly after runs complete

Looking into this, but unable to replicate in the tests yet.

mike-plummer

Looks good in my local testing, going to approve assuming items already identified will be resolved/answered. Just seeing some cleanup items

cli/CHANGELOG.md

packages/app/src/debug/DebugCommitIcon.vue

packages/app/src/debug/useDebugRunSummary.ts

warrensplayer · 2023-03-27T20:29:08Z

@marktnoonan I fixed the issue with the time counting up not stopping when the run completes that you noted in your loom. Can you give it another test?

astone123

Everything looked good in my local testing. nice 🎉

cli/CHANGELOG.md

Co-authored-by: Adam Stone-Lord <adams@cypress.io>

lmiller1990

✅

lmiller1990 · 2023-03-27T23:56:25Z

packages/app/src/debug/DebugContainer.vue

@@ -182,22 +194,30 @@ const props = withDefaults(defineProps<{
  isLoading?: boolean
  commitsAhead?: number
  online?: boolean
+  currentCommitInfo?: InstanceType<typeof DebugRunNavigation>['$props']['currentCommitInfo'] | null


Hadn't seen this to extract the prop types, neat trick!

lmiller1990 · 2023-03-27T23:58:29Z

packages/app/src/pages/Debug.vue


  return relevantRunsHaveNotLoaded || (currentRunIsChanging && (queryIsBeingFetched || waitingForRunToFetchFromTheCloud))
 })

 watchEffect(() => {
+  //console.log('query for debug', query.data.value, relevantRuns.value.commitShas)


Should we remove this? Surprised linting is okay.

lmiller1990 marked this pull request as draft March 6, 2023 04:41

Base automatically changed from stokes/25759_debug_running_builds to develop March 6, 2023 23:56

lmiller1990 force-pushed the lmiller/issue-25899_debug_navigation branch from 098d920 to ad04e03 Compare March 7, 2023 00:08

lmiller1990 added 3 commits March 7, 2023 12:04

wip

bb31bec

wip

ebac150

add some tests

a72fb46

lmiller1990 force-pushed the lmiller/issue-25899_debug_navigation branch from ad04e03 to a72fb46 Compare March 7, 2023 05:16

tests

04d3541

lmiller1990 marked this pull request as ready for review March 7, 2023 06:54

lmiller1990 added 3 commits March 7, 2023 17:52

wip

292ee3e

changing runs

e312f92

update ui

48db51b

lmiller1990 changed the title ~~[WIP] feat: debug navigation backend work~~ [WIP] feat: debug navigation Mar 9, 2023

lmiller1990 added 10 commits March 9, 2023 19:37

wip

b77368e

update

c9cedb2

rename

d6e06fe

update tests

435a4de

refactor

9836f35

refactor to not require next variable

5a167c8

clean up code

7918d3a

improve style

239ebd8

lint

0a4f33a

update tests

c7eca72

warrensplayer reviewed Mar 12, 2023

View reviewed changes

lmiller1990 and others added 5 commits March 13, 2023 09:17

Merge remote-tracking branch 'origin/develop' into lmiller/issue-2589…

4bc7335

…9_debug_navigation

lint

e76574b

wip

46fb432

use frontend to manage selected run

3d95f88

Do not push duplicate shas for allRuns

76002ca

warrensplayer added 10 commits March 26, 2023 13:41

Fix import

2b9da41

add useToggle

aac3d42

Adding transition

684bfbc

Handling promises

f5db2a8

Styling fixes

d14cde6

More full height changes

6355348

Copy value when caching

73bd492

Fixing tests

b7585b4

Documentation and additional test

6dd83a2

Update changelog

00edbb4

lmiller1990 commented Mar 27, 2023

View reviewed changes

Adding documentation

24d8445

warrensplayer marked this pull request as ready for review March 27, 2023 12:44

warrensplayer added 2 commits March 27, 2023 10:00

Update changelog

1b6f81d

Removing unneeded padding

1fbde2d

Merge branch 'develop' into lmiller/issue-25899_debug_navigation

140e534

marktnoonan reviewed Mar 27, 2023

View reviewed changes

mike-plummer approved these changes Mar 27, 2023

View reviewed changes

cli/CHANGELOG.md Outdated Show resolved Hide resolved

packages/app/src/debug/DebugCommitIcon.vue Show resolved Hide resolved

packages/app/src/debug/useDebugRunSummary.ts Outdated Show resolved Hide resolved

packages/app/src/debug/useDebugRunSummary.ts Outdated Show resolved Hide resolved

warrensplayer added 2 commits March 27, 2023 16:15

Remove old comments

e28ea4d

Make sure countup for run duration stops after run becomes complete

c65b4b1

warrensplayer requested a review from marktnoonan March 27, 2023 20:29

astone123 approved these changes Mar 27, 2023

View reviewed changes

cli/CHANGELOG.md Outdated Show resolved Hide resolved

Update cli/CHANGELOG.md

66edf02

Co-authored-by: Adam Stone-Lord <adams@cypress.io>

lmiller1990 commented Mar 28, 2023

View reviewed changes

warrensplayer merged commit 24194da into develop Mar 28, 2023

warrensplayer deleted the lmiller/issue-25899_debug_navigation branch March 28, 2023 00:10

warrensplayer mentioned this pull request Mar 29, 2023

Debug Run Navigation - More than 100 runs message #26288

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Debug run navigation #26017

feat: Debug run navigation #26017

lmiller1990 commented Mar 6, 2023 •

edited by warrensplayer

cypress bot commented Mar 6, 2023 •

edited

warrensplayer left a comment

warrensplayer Mar 12, 2023

lmiller1990 Mar 12, 2023

warrensplayer Mar 12, 2023

warrensplayer Mar 12, 2023

lmiller1990 Mar 13, 2023

lmiller1990 left a comment

lmiller1990 Mar 27, 2023

lmiller1990 Mar 27, 2023

warrensplayer Mar 27, 2023

lmiller1990 Mar 27, 2023

lmiller1990 Mar 27, 2023

lmiller1990 Mar 27, 2023

warrensplayer Mar 27, 2023

marktnoonan commented Mar 27, 2023 •

edited

marktnoonan Mar 27, 2023 •

edited

marktnoonan Mar 27, 2023

warrensplayer Mar 27, 2023

warrensplayer commented Mar 27, 2023 •

edited

mike-plummer left a comment

warrensplayer commented Mar 27, 2023

astone123 left a comment

lmiller1990 left a comment

lmiller1990 Mar 27, 2023

lmiller1990 Mar 27, 2023

	return latest.value?.commitInfo?.sha === current.value?.commitInfo?.sha
	return latest.value?.runNumber === current.value?.runNumber


		const selections = newFieldNode.selectionSet?.selections!

		return {

feat: Debug run navigation #26017

feat: Debug run navigation #26017

Conversation

lmiller1990 commented Mar 6, 2023 • edited by warrensplayer

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress bot commented Mar 6, 2023 • edited

Passing run #44988 ↗︎

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Note for reviewers

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marktnoonan commented Mar 27, 2023 • edited

marktnoonan Mar 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warrensplayer commented Mar 27, 2023 • edited

mike-plummer left a comment

Choose a reason for hiding this comment

warrensplayer commented Mar 27, 2023

astone123 left a comment

Choose a reason for hiding this comment

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 commented Mar 6, 2023 •

edited by warrensplayer

cypress bot commented Mar 6, 2023 •

edited

marktnoonan commented Mar 27, 2023 •

edited

marktnoonan Mar 27, 2023 •

edited

warrensplayer commented Mar 27, 2023 •

edited