-
Notifications
You must be signed in to change notification settings - Fork 25
DEVPROD-1101: Update previous commits selector #2120
Conversation
1 failed test on run #13634 ↗︎
Details:
cypress/integration/task/test_table.ts • 1 failed test
Review all test suite changes for PR #2120 ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job! 🥰
No previous versions available. | ||
</Tooltip> | ||
) : ( | ||
<Menu trigger={<Button size={Size.Small}>Previous commits</Button>}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
: `There is no version that satisfies this criteria.`} | ||
</Tooltip> | ||
)} | ||
{versionMetadata?.isPatch ? "Base" : "Previous"} commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// We don't error for this query because it is the default query that is run when the page loads. | ||
// If it errors it probably means there is no base version, which is fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can delete this comment since there's a skip
on this now :0 (doesn't run by default)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yayyyy bye 👋
|
||
return { | ||
[CommitType.Base]: getTaskRoute(parentTask.id), | ||
[CommitType.LastPassing]: getTaskRoute( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link would always default to the parent task even if it does not pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if I'm misunderstanding, doesn't this test case assert that the last passing task works correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looping back I missed that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM mod one comment
|
||
return { | ||
[CommitType.Base]: getTaskRoute(parentTask.id), | ||
[CommitType.LastPassing]: getTaskRoute( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looping back I missed that!
.buildVariants ?? []; | ||
if (buildVariants.length > 1) { | ||
reportError( | ||
new Error("Multiple build variants matched previous commit search.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there more info here that we can add to the sentry search? Or will sentry give us the necessary data be default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentry will include a breadcrumb with the GraphQL query sent before this error. I think that's enough information to start investigating this.
DEVPROD-1101 - This is what I worked on over skunkworks because it's always irked me 😁
Description
Screenshots
Testing