Skip to content

Conversation

@lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Dec 10, 2024

Edit: This seems to only affect npm run test-packaged, not npm run test-packaged-ci which is what we run in CI. So it was fine in CI, just not locally. Depending on which command you run locally.

Since the refactor a couple of months ago we've been passing too many -- to the app and the arg parser was missing --test-packaged-app so it would re-build compass every time.

You can test this by running the E2E tests against the locally installed beta, dev or GA like this:

DEBUG=compass-e2e-tests*,electron*,hadron*,mongo* COMPASS_APP_NAME="MongoDB Compass" COMPASS_APP_PATH="/Applications/MongoDB Compass.app"  npm run test-packaged

@lerouxb lerouxb added the no release notes Fix or feature not for release notes label Dec 10, 2024
@github-actions github-actions bot added the fix label Dec 10, 2024
export const context = parsedArgs as CommonParsedArgs &
Partial<DesktopParsedArgs & WebParsedArgs>;

debug('context:', context);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handy for seeing if it actually picked up all the command-line args it should have.

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2024-12-10 at 12 23 46 PM

Nice add

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's printed already a few lines down with secrets removed from the list:

const contextForPrinting = Object.fromEntries(
  Object.entries(context).map(([k, v]) => {
    return [k, /password/i.test(k) ? '<secret>' : v];
  })
);
debug('Running tests with the following arguments:', contextForPrinting);

Is this print doing something different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry didn't see it. We can remove it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just merged it #6561

@lerouxb lerouxb added the no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) label Dec 10, 2024
@lerouxb lerouxb changed the title fix(e2e): actually test the packaged app fix(e2e): actually test the packaged app when running npm run test-packaged-app Dec 10, 2024
@lerouxb lerouxb changed the title fix(e2e): actually test the packaged app when running npm run test-packaged-app fix(e2e): actually test the packaged app when running npm run test-packaged Dec 10, 2024
@lerouxb lerouxb merged commit ba50fcd into main Dec 10, 2024
26 of 28 checks passed
@lerouxb lerouxb deleted the actually-test-packaged-app branch December 10, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants