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.

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

CircleCI checks - Fix running Full tests twice in certain branches #5478

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

geriux
Copy link
Member

@geriux geriux commented Feb 15, 2023

This is a follow-up of #5462 where it was causing an issue where the Test iOS on Device - Full and Test Android on Device - Full steps would get triggered for release, dependabot, and trunk branches when it shouldn't, as there are steps specifically for those branches.

Making the Optional UI Tests as required and that requirement being ignored for those branches would trigger full tests to run twice. This PR addresses that issue.

You could see this happening in the trunk branch after the recent merge.

To test
CI checks should pass.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@geriux geriux added the Testing Anything related to automated tests label Feb 15, 2023
@geriux geriux requested a review from fluiddot February 15, 2023 16:10
@geriux geriux changed the title CircleCI steps - Fix avoid running full E2E test twices in release, d… CircleCI steps - Fix avoid running full E2E test twice Feb 15, 2023
@geriux geriux changed the title CircleCI steps - Fix avoid running full E2E test twice CircleCI checks - Fix running Full tests twice in certain branches Feb 15, 2023
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

LGTM 🎊 !

Ah, good catch. I completely overlooked this issue when reviewing #5462. Thanks for fixing it @geriux 🙇 !

On a different note, I noticed that we have a bit of redundancy on the jobs for trunk, dependabot, and release branches (reference). Not sure why we have one job for each one but seems we could unify them into one using the only filter, I'll create a quick PR as a follow-up on this.

@geriux
Copy link
Member Author

geriux commented Feb 16, 2023

Ah, good catch. I completely overlooked this issue when reviewing #5462. Thanks for fixing it @geriux 🙇 !

No worries I thought that by having the requirement was enough 😅

On a different note, I noticed that we have a bit of redundancy on the jobs for trunk, dependabot, and release branches (reference). Not sure why we have one job for each one but seems we could unify them into one using the only filter, I'll create a quick PR as a follow-up on this.

Yeah, I agree! Thanks for working on that!

@geriux geriux merged commit 6a5b91a into trunk Feb 16, 2023
@geriux geriux deleted the fix-filtering-full-e2e-steps branch February 16, 2023 10:13
@fluiddot fluiddot added [Type] Enhancement Improves a current area of the editor and removed [Type] Enhancement Improves a current area of the editor labels Feb 16, 2023
@fluiddot
Copy link
Contributor

On a different note, I noticed that we have a bit of redundancy on the jobs for trunk, dependabot, and release branches (reference). Not sure why we have one job for each one but seems we could unify them into one using the only filter, I'll create a quick PR as a follow-up on this.

Here is the PR. @geriux I assigned you as a reviewer, the change is an enhancement so please don't prioritize the review. Thanks 🙇 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing Anything related to automated tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants