Skip to content

Conversation

cniackz
Copy link
Collaborator

@cniackz cniackz commented Mar 23, 2023

Objective:

To make playwright run faster by running tests in port 9090 that is already up and running from previous steps. And since console on port 9090 is needed anyway then we can skip 5005 and save time, here a diagram on how this works: https://github.com/cniackz/public/wiki/How-Playwright-Coverage-Works-in-GitHub-On-Console-Repo

  1. I need to make sure, we can get coverage: For the coverage, I need to create another build that injects or contains the Istambul puglin for coverage to be retrieved, example:
"buildistanbulcoverage": "PORT=9090 USE_BABEL_PLUGIN_ISTANBUL=1 react-app-rewired build",

It should look like:

npx nyc report
---------------------------------------|---------|----------|---------|---------|-------------------
File                                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
---------------------------------------|---------|----------|---------|---------|-------------------
All files                              |   43.88 |    32.87 |    27.7 |   43.92 |                   
 src                                   |    50.9 |    34.14 |   35.59 |   50.31 |                   
  MainRouter.tsx                       |    62.5 |      100 |      25 |   83.33 | 27                
  ProtectedRoutes.tsx                  |      66 |    51.42 |   53.84 |    65.3 | ...70-106,1[20](https://github.com/minio/console/actions/runs/4519423200/jobs/7961393176?pr=2737#step:8:21),134
  1. I need to make sure tests can run in 9090 so they re-use the UI running in the compiled binary of console and we save time.
Running 6 tests using 1 worker
······
  6 passed
  1. In another PR, I will address the issue or improvement of having an env variable for switching between 5005 and 9090 but for now let's keep as it is for simplicity of this change and then we can improve that.

Additional information:

  • Failure in SSO is not related to this change:
adding policy to Dillon Harper to be able to login:
mc: <ERROR> Incorrect command. Please use 'mc admin policy create'.
make: *** [Makefile:147: test-sso-integration] Error 1
Error: Process completed with exit code 2.
##[debug]Finishing: Build on 

Testing and comparison:

  • With this change we are saving ~2 min per execution:
succeeded 4 hours ago in 4m 46s

versus

succeeded 2 days ago in 6m 23s

@cniackz cniackz self-assigned this Mar 23, 2023
@cniackz cniackz force-pushed the make-playwright-faster branch from 05311c4 to 8256987 Compare March 23, 2023 16:28
@cniackz cniackz added the WIP This PR is WIP and cannot be merged yet label Mar 23, 2023
@cniackz cniackz force-pushed the make-playwright-faster branch 3 times, most recently from caaf781 to f8fdead Compare March 24, 2023 22:41
@cniackz
Copy link
Collaborator Author

cniackz commented Mar 24, 2023

I think I can get this tomorrow morning. And yes, we will save time here!. 💪

@cniackz cniackz force-pushed the make-playwright-faster branch 3 times, most recently from 969369e to 9f43602 Compare March 25, 2023 13:14
@cniackz cniackz changed the title [WIP] - Make playwright run faster Make playwright run faster Mar 25, 2023
@cniackz cniackz removed the WIP This PR is WIP and cannot be merged yet label Mar 25, 2023
@cniackz cniackz force-pushed the make-playwright-faster branch 2 times, most recently from e55c298 to 4fb4e70 Compare April 24, 2023 21:59
@cniackz cniackz requested a review from prakashsvmx April 24, 2023 21:59
…avoiding to run on port 5005 that can be skipped if we incorporate istanbul plugin on the assets since the start and then coverage can be obtained from port 9090 instead saving time.
@cniackz cniackz force-pushed the make-playwright-faster branch from b56d727 to 8eb5209 Compare April 25, 2023 12:58
Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@dvaldivia dvaldivia merged commit 6020590 into minio:master Apr 25, 2023
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.

5 participants