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

Run UI e2e tests with given aerie/gateway docker tags when provided in PR body #1439

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

dandelany
Copy link
Collaborator

@dandelany dandelany commented Aug 21, 2024

___REQUIRES_AERIE_PR___="1540"
___REQUIRES_GATEWAY_PR___="107"

Description

Supporting this issue: NASA-AMMOS/aerie#1538

This PR allows the aerie-ui PR e2e test suite to use different Docker image tags, published by PR(s) on their respective repos, for the aerie & aerie-gateway containers when testing, instead of develop as they usually do. The aerie/gateway PR(s) must first be opened and labeled with the "publish" label in github, which will cause them to publish images tagged eg. pr-1540. (see NASA-AMMOS/aerie#1540 & NASA-AMMOS/aerie-gateway#107 )

Once this is done, you can add the flag(s) at the top of this PR (___REQUIRES_AERIE_PR___ and/or ___REQUIRES_GATEWAY_PR___) to your aerie-ui PR to indicate the PR branches to be tested with.

Verification

This PR itself demonstrates the workflow:

  • The Test workflow re-runs when the PR body is modified (to check for flags)
  • The flags in the PR body are parsed by the workflow
  • You can see in the workflow logs that the correct docker image tags were pulled (pr-1540 for aerie and pr-107 for gateway)
  • I've also tested removing these flags to make sure the workflow still works correctly for PRs not passing them

@dandelany dandelany changed the title WIP: improvements to GH workflows Run UI e2e tests with given aerie/gateway docker tags when provided in PR body Aug 28, 2024
@dandelany dandelany marked this pull request as ready for review August 28, 2024 01:43
@dandelany dandelany requested a review from a team as a code owner August 28, 2024 01:43
@dandelany dandelany force-pushed the dev/gh-test-workflows branch from 01c11f6 to 5bbc506 Compare August 28, 2024 15:57
@dandelany dandelany removed request for a team, AaronPlave, duranb and joswig August 28, 2024 15:57
Copy link
Contributor

@skovati skovati left a comment

Choose a reason for hiding this comment

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

Another (possibly simpler) architecture we might want to consider for this is just cloning the aerie repo into the e2e-test GH action workspace and building images (and mission models!) from scratch for each test run.

This would remove the need to make any changes to the backend repo's GH actions, i.e. no need to run publish or add custom tags. The UI and gateway could just have a tag / command in the PR body that tell it which aerie branch to checkout.

This would also solve the problem of having stale mission model jars in the UI repo.

@dandelany
Copy link
Collaborator Author

Thanks @skovati - I considered that too... I think I'd prefer to keep the process of "how to build aerie backend" out of this UI repo if possible, from a separation of concerns/maintainability perspective. This way gives me slightly better assurance that the backend images I'm testing with will be exactly the same as the final published images, since they're created via the same process in the aerie repo. If we built here, we'd have to keep the build process in sync with aerie, and if we miss a small change to the process, we might be testing with a subtly different build than the final aerie build. We'd also have to do the same thing for aerie-gateway.

I think if you're OK with this method, let's do it this way for now, but I'm definitely open to talking about improving this process/general workflow for our different repos in future.

@Mythicaeda
Copy link
Contributor

For your tests, have you tested only having a Gateway PR and not the Backend and vice versa?

@dandelany
Copy link
Collaborator Author

dandelany commented Aug 28, 2024

For your tests, have you tested only having a Gateway PR and not the Backend and vice versa?

Yep! I tested backend without gateway yesterday, I just tested gateway without backend now and that appears to work correctly too:
image
Just switched back to including both and it re-ran as expected.

Copy link
Contributor

@Mythicaeda Mythicaeda left a comment

Choose a reason for hiding this comment

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

Please clean up the commit history before merging, but the work itself LGTM

…E_PR and REQUIRES_GATEWAY_PR flags and use tagged docker images from specified PR if so
@dandelany
Copy link
Collaborator Author

Please clean up the commit history before merging, but the work itself LGTM

Thanks - done on this and other PRs. Will merge when tests pass again 👍

Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

Thanks for figuring this out!

@dandelany dandelany merged commit 4e31f20 into develop Aug 29, 2024
5 checks passed
@dandelany dandelany deleted the dev/gh-test-workflows branch August 29, 2024 00:38
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
…n PR body (#1439)

GH workflows: update Test workflow to check PR body for REQUIRES_AERIE_PR and REQUIRES_GATEWAY_PR flags and use tagged docker images from specified PR if so
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants