fix(plugin-webpack): try other logger ports
- [x] I have read the contribution documentation for this project.
- [x] I agree to follow the code of conduct that this project follows, as appropriate.
- [x] The changes are appropriately documented (if applicable).
- [x] The changes have sufficient test coverage (if applicable).
- [x] The testsuite passes successfully on my local machine (if applicable).
Summarize your changes: Fixes #3204
- Checks for availability of ports from 9000-9010.
- Shows which port has been assigned.
- Throws an appropriate error message for better user experience.
Currently I am only checking till port 9009, @erickzhao can you guide me if we should increase the upper limit for this port availability checking.
Hi @BlackHole1, just made the changes that you suggested in code review and wrote unit tests for Logger.ts. I couldn't directly test the private functions such as portOccupied and findAvailablePort, so I wrote test for start() such that the private functions are tested without changing Access levels.
Thank you for the code review, I hope this fixes the issue. Glad to Contribute.
Can you move portOccupied and findAvailablePort to packages/utils/core-utils/src?
- Create a file port.ts in the packages/utils/core-utils/src directory.
- Move
portOccupiedandfindAvailablePortto port.ts as arrow functions (not as class members). - Add a file port_spec.ts in the packages/utils/core-utils/test directory and write unit tests.
Note:
Currently one of the tests that I have written are dependent on port 49152 i.e if that port is occupied one my test will fail. We can get rid of this dependency by removing the tests that use 49152 because the tests that check for rejection are indirectly checking for these test.
I am new to writing tests so I might be wrong
When I updated the current branch, the CI failed, it seems to be related to Code Lint. @DevanceJ, can you try to fix it?
Hi Kevin, I just saw this. Working on it now!
Looking at the time the tests take, can you check my pr adding parallelism in circleci/config #3536.😁
Looking at the time the tests take, can you check my pr adding parallelism in circleci/config #3536.😁
I am not very familiar with CircleCI, so #3536 may need to be reviewed by @dsanders11 and @georgexu99.
Hey, I have fixed the CI errors in the previous merge from main.
Hi Erick, I have Refactored portOccupied function for clarity and reliability. Updated test cases accordingly.
Looking into this error.
Hi I am getting this error
TSError: ⨯ Unable to compile TypeScript: packages/plugin/webpack/src/WebpackPlugin.ts:574:42 - error TS2554: Expected 0 arguments, but got 1. 574 this.loggerPort = await logger.start(this.loggerPort as number);
when I run yarn test locally but the start method does expect a parameter in reality. Any help on this is appreciated.
Hi sorry for the delay, I believe there was a naming conflict with the start function.
@erickzhao please review this. Thank you