Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: fix component tests in contributor flow #27883

Merged
merged 16 commits into from
Oct 10, 2023
Merged

chore: fix component tests in contributor flow #27883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0220a06
chore: update contributor workflow
young-robot Sep 14, 2023
76f8319
Merge remote-tracking branch 'upstream/develop' into develop
young-robot Sep 20, 2023
0cb711f
Merge remote-tracking branch 'upstream/develop' into develop
young-robot Sep 20, 2023
4adb2fd
Merge remote-tracking branch 'upstream/develop' into develop
young-robot Sep 21, 2023
8663014
Merge remote-tracking branch 'upstream/develop' into develop
young-robot Sep 22, 2023
22c407e
fix: add optional chaining for prevAttempts
young-robot Sep 22, 2023
5105e9b
don't concat undefined
young-robot Sep 22, 2023
9a83855
add null check for delete
young-robot Sep 22, 2023
9a96dfe
no need to exclude these anymore
young-robot Sep 22, 2023
df4f828
Merge branch 'develop' into optional-chain-prev-attempts
astone123 Sep 26, 2023
37150e5
Merge branch 'develop' into optional-chain-prev-attempts
dkasper-was-taken Sep 29, 2023
fcf6993
Merge branch 'develop' into optional-chain-prev-attempts
dkasper-was-taken Sep 29, 2023
f3ed7e8
Merge branch 'develop' into optional-chain-prev-attempts
dkasper-was-taken Sep 29, 2023
77406bc
Merge branch 'develop' into optional-chain-prev-attempts
dkasper-was-taken Oct 3, 2023
d8e1d15
Merge branch 'develop' into optional-chain-prev-attempts
dkasper-was-taken Oct 4, 2023
b092358
Merge branch 'develop' into optional-chain-prev-attempts
dkasper-was-taken Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/workflows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ commands:
if [[ <<parameters.type>> == 'ct' ]]; then
# component tests are located side by side with the source codes.
# for the app component tests, ignore specs that are known to cause failures on contributor PRs (see https://discuss.circleci.com/t/how-to-exclude-certain-files-from-circleci-test-globbing/41028)
TESTFILES=$(find src -regextype posix-extended -name '*.cy.*' -not -regex '.*(FileMatch|PromoAction|SelectorPlayground|useDurationFormat|useTestingType|SpecPatterns|DebugPendingRunCounts|DebugRunStates|DebugPageHeader|DebugPendingRunSplash|DebugRunNavigation|DebugRunNavigationLimitMessage).cy.*' | circleci tests split --total=$CIRCLE_NODE_TOTAL)
Copy link
Contributor

@astone123 astone123 Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably remove even more of these since the error is fixed now - most of them were excluded because of this issue I believe. I want to do that in a different PR though, since we can't test the contributor workflow until it's merged into develop

TESTFILES=$(find src -regextype posix-extended -name '*.cy.*' -not -regex '.*(FileMatch|PromoAction|SelectorPlayground|useDurationFormat|useTestingType|SpecPatterns).cy.*' | circleci tests split --total=$CIRCLE_NODE_TOTAL)
else
GLOB="cypress/e2e/**/*cy.*"
TESTFILES=$(circleci tests glob "$GLOB" | circleci tests split --total=$CIRCLE_NODE_TOTAL)
Expand Down
8 changes: 5 additions & 3 deletions packages/driver/src/cypress/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1342,11 +1342,13 @@ export default {
const replacePreviousAttemptWith = (test) => {
const prevAttempt = _testsById[test.id]

const prevAttempts = prevAttempt.prevAttempts || []
const prevAttempts = prevAttempt?.prevAttempts || []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiousity, why did you need to touch this code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code was throwing errors in the app component tests job for some reason - see the link to the failure in the PR description. This is what fixes those tests in the contributor workflow

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. Are those component tests retrying? I'm just trying to figure out why that would happen because there should be a previous attempt here if the currentRetry is 1 or more. I'm more so concerned there is some time of regression here, but I have no idea why this would be specifically on contributor PRs. any idea? I'm also OK with patching this after we get builds working it isn't a must to figure out right now imo


const newPrevAttempts = prevAttempts.concat([prevAttempt])
const newPrevAttempts = prevAttempt ? prevAttempts.concat([prevAttempt]) : prevAttempts

delete prevAttempt.prevAttempts
if (prevAttempt) {
delete prevAttempt.prevAttempts
}

test.prevAttempts = newPrevAttempts

Expand Down
Loading