Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

chore/e2e lavamoat enabled#418

Merged
racitores merged 14 commits intomainfrom
chore/e2e-lavamoat-enabled
Jan 27, 2023
Merged

chore/e2e lavamoat enabled#418
racitores merged 14 commits intomainfrom
chore/e2e-lavamoat-enabled

Conversation

@racitores
Copy link
Copy Markdown
Contributor

@racitores racitores commented Jan 16, 2023

Explanation

This ticket implements Run existing e2e tests with LavaMoat enabled

Screenshots/Screencaps

Before

After

Manual Testing Steps

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@racitores racitores force-pushed the chore/e2e-lavamoat-enabled branch 3 times, most recently from 859cc2d to 4b1dd4d Compare January 17, 2023 08:56
@racitores racitores changed the title Chore/e2e lavamoat enabled chore/e2e lavamoat enabled Jan 17, 2023
@racitores racitores marked this pull request as ready for review January 17, 2023 11:08
@racitores racitores requested a review from a team January 17, 2023 11:08
@racitores racitores force-pushed the chore/e2e-lavamoat-enabled branch from fe6b5f6 to 11cfa55 Compare January 17, 2023 14:17
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the confusion here, while I don't think the test builds need non-lavamoat versions, I think the UI is fundamental enough that we'll still want to be able to build without Lavamoat.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This ☝️. We still need to have both build:ui and build:ui:lavamoat. It is just that we are using build:ui:lavamoat for testing purposes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't have to change as the build itself doesn't change, Lavamoat being used is based on which script we start it with. So the E2E tests would instead use start:lavamoat.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as for the extension, this doesn't change but we run the app using start:lavamoat.

Copy link
Copy Markdown
Member

@matthewwalsh0 matthewwalsh0 Jan 20, 2023

Choose a reason for hiding this comment

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

This can be removed now as the build:ui:ci is much more consistently named.

@racitores racitores force-pushed the chore/e2e-lavamoat-enabled branch from 2a92051 to 07980e4 Compare January 20, 2023 10:24
@bergarces bergarces force-pushed the chore/e2e-lavamoat-enabled branch from 9d9f39d to 1d321de Compare January 23, 2023 14:44
@bergarces
Copy link
Copy Markdown
Contributor

Before merging this one, we have to merge https://github.com/MetaMask/desktop-extension/pull/22 and update the commitid here to match the latest desktop-extension.

@racitores racitores force-pushed the chore/e2e-lavamoat-enabled branch from 9ec61d5 to 0c50afc Compare January 27, 2023 10:50
@racitores racitores merged commit 972dd1e into main Jan 27, 2023
@racitores racitores deleted the chore/e2e-lavamoat-enabled branch January 31, 2023 18:21
@cryptotavares cryptotavares mentioned this pull request Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants