feat: debug status badge #25152

ZachJW34 · 2022-12-14T18:18:56Z

DRAFT: Need to add spider icon to design system and incorporate

Closes [IATR](M1.0) Debug sidebar status count #24852

User facing changelog

Adds status badge to Debug Sidebar, indicating the results of the latest run.

Additional details

Did some refactoring of the SidebarNavigation.vue component, lifting the queries up to a container component to make the component easier to test.

Steps to test

Driven mostly off of the CT tests, if you have cypress-services configured locally you can:

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?

PR Tasks

Have tests been added/updated?
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-14T18:18:58Z

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.

lmiller1990

This mostly looks good to me, I don't see any obvious issues. The new UI designs sure do look good.

lmiller1990 · 2022-12-15T02:57:24Z

packages/app/src/navigation/SidebarNavigationRow.vue

+watch(() => props.isNavBarExpanded, async () => {
+  if (props.badge) {
+    transitioning.value = true
+    await promiseTimeout(125)


Is there a variable we can share? How did you arrive at 125?

The animations are driven off of tailwind classes like duration-300. 125 felt right, I didn't want it to disappear for too long, just enough for it not to thrash

lmiller1990 · 2022-12-15T02:57:50Z

packages/app/src/navigation/SidebarNavigationContainer.vue

+<template>
+  <SidebarNavigation
+    :gql="query.data.value"
+    :is-loading="query.fetching.value"


Nice refactor 👍

tooling/v8-snapshot/cache/dev-darwin/snapshot-meta.cache.json

lmiller1990 · 2022-12-15T03:03:38Z

packages/app/src/navigation/SidebarNavigation.cy.tsx

+
+      cy.percySnapshot('Debug Badge:errored')
+    })
+


Now we've got Vite 4, we should definitely get Vitest in - testing this complex logic via components makes reviewing a bit challenging. It looks like you've covered all the cases, though, which is great.

We might have to steal some of the gql-testing infra if we implement vitest as this logic is heavily tied to gql

I think we could probably decouple the logic from the UI - and just keep the component tests to make sure it's all wired up correctly. Anyway, another battle for another day.

cypress · 2022-12-15T17:42:35Z

Test summary

67 • 0 • 10 • 0 • 0

Run details


Project	cypress
Status	Passed
Commit	`4c21f5f`
Started	Dec 16, 2022 12:02 AM
Ended	Dec 16, 2022 12:15 AM
Duration	13:45 💡
OS	Linux Debian -
Browser	Chrome 106

View run in 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

…g-status-badge

astone123 · 2022-12-15T18:59:43Z

Not sure if this is in scope but I'm seeing some weird stuff when I load the app (when I'm logged into production cloud). The debug status badge flickers to different statuses for a few seconds and then eventually lands on one. I'm also seeing the banner that says I don't have a connection to the cloud... I think I've been seeing this recently and I'm not sure if it's related at all. Not sure what's going on there

https://www.loom.com/share/cef6b0700f4f45a39ef503ce5d8f764c

warrensplayer

Looks great! Most of my comments are just questions. My suggested change for the messages in en-US.json are the only mandatory changes and updating the tests to go along with that. Going to go ahead and approve expecting that part to be done before merging to the feature branch.

warrensplayer · 2022-12-15T15:43:52Z

packages/app/src/navigation/SidebarNavigation.cy.tsx

+      // Set to February 15, 2023 to see this fail
+      cy.clock(new Date('February 20, 2023'))
+      mountComponent()
+      cy.findByLabelText('New Debug feature').should('not.exist')


Use messages imported fromdefaultMessages instead of hardcoded strings in these tests. I am proposing a change to these strings in the locale file and this would keep future changes in sync

I'm not the biggest fan of this, though this has been discussed in the past. For example, how do I assert that the label is Last recorded run had 1 test failure? Do I write defaultMessages.sidebar.debug.failed.split(' | ')[0].replace('{n}', 1). Tests should be explicit, not have the misdirection of referencing some variable and then having to add logic to make sure it works. I know this is an edge case but the only pro for referencing defaultMessages is that I don't have to update the test when I change the text which is not worth it IMO.

I agree, I find it really difficult to read and search for specs that import the i18n file. If anything, doesn't not importing it give us more coverage? If we change the text in the i18n file, we will intentionally need to update the relevant tests, etc.

packages/frontend-shared/src/locales/en-US.json

warrensplayer · 2022-12-15T20:06:20Z

packages/app/src/navigation/SidebarNavigationRow.vue

@@ -48,6 +47,14 @@
      >
        {{ name }}
      </span>
+      <span


What about pulling this out into its own component? Any reason that it could not stand on its own?

I tried to do this but I couldn't get the animation to work properly due to the component getting destroyed. It's not a lot of logic and makes sense in the context of the row so I'm ok with how it is.

packages/app/src/navigation/SidebarNavigation.vue

packages/frontend-shared/src/utils/isAllowedFeature.ts

packages/app/src/navigation/SidebarNavigation.cy.tsx

astone123 · 2022-12-15T20:20:01Z

yeah, seems like whenever I'm on this feature branch I see that banner saying that there isn't a collection to cloud. Is that expected at this point?

ZachJW34 · 2022-12-15T21:14:42Z

@astone123 I'll look into this a bit, I've seen it as well but I've never been able to reproduce it when I'm connected to my locally running dashboard. Running yarn cypress:open puts you in a weird spot where the app is trying to contact the cloud but the contracts aren't the same

astone123 · 2022-12-15T21:19:02Z

@ZachJW34 yeah that's what it seems like. Probably nothing to worry about then 👍🏻

ZachJW34 added 2 commits December 14, 2022 11:42

feat: debug status badge

ed72e75

remove unused code

b12d4ae

lmiller1990 approved these changes Dec 15, 2022

View reviewed changes

warrensplayer self-requested a review December 15, 2022 15:38

add debug icon

d01e46c

ZachJW34 marked this pull request as ready for review December 15, 2022 17:04

ZachJW34 added 2 commits December 15, 2022 11:58

fix e2e test

a1b6f02

Merge remote-tracking branch 'origin/feature/IATR-M0' into zachw/debu…

85fe272

…g-status-badge

warrensplayer approved these changes Dec 15, 2022

View reviewed changes

address feedback

4c21f5f

lmiller1990 merged commit 05805e0 into feature/IATR-M0 Dec 16, 2022

lmiller1990 deleted the zachw/debug-status-badge branch December 16, 2022 14:11

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

[IATR](M1.0) Debug sidebar status count #24852

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: debug status badge #25152

feat: debug status badge #25152

ZachJW34 commented Dec 14, 2022

cypress-bot bot commented Dec 14, 2022

lmiller1990 left a comment

lmiller1990 Dec 15, 2022

ZachJW34 Dec 15, 2022

lmiller1990 Dec 15, 2022

lmiller1990 Dec 15, 2022

ZachJW34 Dec 15, 2022

lmiller1990 Dec 15, 2022

cypress bot commented Dec 15, 2022 •

edited

Loading

astone123 commented Dec 15, 2022

warrensplayer left a comment

warrensplayer Dec 15, 2022

ZachJW34 Dec 15, 2022 •

edited

Loading

lmiller1990 Dec 15, 2022

warrensplayer Dec 15, 2022

ZachJW34 Dec 15, 2022

astone123 commented Dec 15, 2022

ZachJW34 commented Dec 15, 2022

astone123 commented Dec 15, 2022

feat: debug status badge #25152

feat: debug status badge #25152

Conversation

ZachJW34 commented Dec 14, 2022

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Dec 14, 2022

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

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

cypress bot commented Dec 15, 2022 • edited Loading

Test summary

Run details

astone123 commented Dec 15, 2022

warrensplayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZachJW34 Dec 15, 2022 • edited Loading

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 15, 2022

ZachJW34 commented Dec 15, 2022

astone123 commented Dec 15, 2022

cypress bot commented Dec 15, 2022 •

edited

Loading

ZachJW34 Dec 15, 2022 •

edited

Loading