Skip to content

Module manager test #994

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

Merged
merged 3 commits into from
May 20, 2025
Merged

Conversation

govza
Copy link
Contributor

@govza govza commented May 19, 2025

* denotes required fields

Priority*

  • High: This PR needs to be merged first, before other tasks.
  • Medium: This PR should be merged quickly to prevent conflicts due to common changes. (default)
  • Low: This PR does not affect other tasks, so it can be merged later.

Purpose of the PR*

Run tests for separated features

Changes*

Created e2e-moduler GH workflow

How to check the feature

  1. pnpm module-manager --de tests random-module
  2. pnpm run e2e
  3. pnpm module-manager -r random-module
  4. pnpm run e2e

Closes #937

@govza govza requested a review from Jonghakseo as a code owner May 19, 2025 00:53
@auto-assign auto-assign bot requested a review from PatrykKuniczak May 19, 2025 00:53
Copy link
Collaborator

@PatrykKuniczak PatrykKuniczak left a comment

Choose a reason for hiding this comment

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

What determines the current browser for pnpm e2e ?

Browser on which we're currently are or what?

And tests run will iterate on both browsers and run tests for both?

And fix CodeQL issues.

And it isn't possible to run it on local machine, only on CI, are you think about to add it also as a package.json script?

@govza govza force-pushed the Module-manager-test branch from a19814b to 44c590f Compare May 19, 2025 18:19
@govza govza requested a review from PatrykKuniczak May 19, 2025 18:26
@govza
Copy link
Contributor Author

govza commented May 19, 2025

What determines the current browser for pnpm e2e ?

Browser on which we're currently are or what?

And tests run will iterate on both browsers and run tests for both?

And fix CodeQL issues.

And it isn't possible to run it on local machine, only on CI, are you think about to add it also as a package.json script?

pnpm e2e runs by default on chrome, pnpm e2e:firefox runs on firefox
the matrix strategy defined in workflow allows to run them altogether in parallel,
It's possible to run it locally since it's just - remove all module, - run e2e, - restore random module, - run e2e,
but I don't see any use-case for it.

IDK how to trigger them here, before merge, but they were running on my fork
https://github.com/govza/chrome-extension-boilerplate-react-vite/actions/runs/15101977726

@PatrykKuniczak
Copy link
Collaborator

@govza I'm also wondering about use case of local run, and i think we can skip it, because it should work only on CI.

And why it isn't working here, i don't freaking know, i was trying everything to get knowledge, why it can't work.
I see you have pull_request_target and i think it should work, let's set permissions: write-all for test, and check it then.

But overall there's something wrong with settings of repo, because on my fork it also works better.
@Jonghakseo need to take a look on that problem deeper.

@govza govza requested a review from PatrykKuniczak May 20, 2025 13:44
Copy link
Collaborator

@PatrykKuniczak PatrykKuniczak left a comment

Choose a reason for hiding this comment

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

LGTM very good work :)

@PatrykKuniczak PatrykKuniczak linked an issue May 20, 2025 that may be closed by this pull request
@PatrykKuniczak PatrykKuniczak added the enhancement New feature or request label May 20, 2025
@PatrykKuniczak PatrykKuniczak merged commit 60014ba into Jonghakseo:dev May 20, 2025
10 checks passed
PatrykKuniczak pushed a commit that referenced this pull request May 21, 2025
PatrykKuniczak pushed a commit that referenced this pull request May 21, 2025
PatrykKuniczak pushed a commit that referenced this pull request May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for module-manager
2 participants