Skip to content
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

[WIP] chore: migrate to commander #1347

Closed
wants to merge 15 commits into from

Conversation

rishabh3112
Copy link
Member

@rishabh3112 rishabh3112 commented Mar 18, 2020

Refers #717

What kind of change does this PR introduce?
Migration to new arguments parser

Did you add tests for your changes?
No, needs verification

If relevant, did you update the documentation?
No, ideally shouldn't be needed

Summary
Use commander as decided at #717 (comment)

Does this PR introduce a breaking change?
Don't mean to, attempting as seamless transition as possible

Other information
Tasks needed to be done.

  • Handle custom types ( flag values should be sanitised where their functionality is implemented )
  • Migrate serve package to use commander.js
  • Add support for webpack serve help
  • Verify tests
  • Remove command-line-args completely
  • Process global flag correctly ( cli-team decided to drop support for it.)

@webpack-bot
Copy link

@rishabh3112 The tests look fine, but there are code style issue in your Pull Request. Please review the following:

Worker information [...]
Build system information [...]
travis_fold:start:docker_mtu
travis_fold:end:docker_mtu
travis_fold:start:resolvconf
travis_fold:end:resolvconf


Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions

Setting environment variables from repository settings
$ export nproc=4

Setting environment variables from .travis.yml
$ export JOB_PART=lint


Setting up build cache [...]
 [...]
$ node --version
v12.16.1
$ npm --version
6.13.4
$ nvm --version
0.35.3
$ yarn --version
1.15.2

$ yarn travis:lint
�[2K�[1Gyarn run v1.22.4
�[2K�[1G$ yarn build && yarn lint
�[2K�[1G$ node scripts/buildPackages.js
�[2K�[1G$ /home/travis/build/webpack/webpack-cli/node_modules/.bin/tsc -b /home/travis/build/webpack/webpack-cli/packages/generate-loader /home/travis/build/webpack/webpack-cli/packages/generate-plugin /home/travis/build/webpack/webpack-cli/packages/generators /home/travis/build/webpack/webpack-cli/packages/info /home/travis/build/webpack/webpack-cli/packages/init /home/travis/build/webpack/webpack-cli/packages/migrate /home/travis/build/webpack/webpack-cli/packages/package-utils /home/travis/build/webpack/webpack-cli/packages/serve /home/travis/build/webpack/webpack-cli/packages/utils /home/travis/build/webpack/webpack-cli/packages/webpack-scaffold
 Successfully built TypeScript definition files 
�[2K�[1G$ eslint . --ext .js,.ts
�[2K�[1GDone in 11.54s.
The command "yarn travis:lint" exited with 0.
$ yarn test:ci
�[2K�[1Gyarn run v1.22.4
�[2K�[1G$ yarn test:cli && yarn test:packages
�[2K�[1G$ nyc jest test/ --maxWorkers=4 --reporters=default --reporters=jest-junit
 PASS  test/node/node.test.js (9.479s)
 PASS  test/utils/test-utils.test.js
 PASS  test/output/named-bundles/output-named-bundles.test.js (19.843s)
 PASS  test/no-mode/no-mode.test.js (22.782s)
 PASS  test/info/info-help.test.js (16.091s)
 PASS  test/mode/prod/prod.test.js (28.431s)
 PASS  test/mode/dev/dev.test.js (15.156s)
 PASS  test/help/help-single-arg.test.js (12.629s)
 PASS  test/version/version-multi-args.test.js (15.411s)
 PASS  test/defaults/output-defaults.test.js (15.625s)
 PASS  test/serve/serve-basic.test.js (24.561s)
 PASS  test/info/info-output.test.js (21.287s)
 PASS  test/stats/stats.test.js (48.247s)
 PASS  test/help/help-flags.test.js (9.271s)
 PASS  test/info/info-multi-args.test.js (17.881s)
 PASS  test/source-map/object/source-map-object.test.js (13.693s)
 PASS  test/help/help-commands.test.js (9.302s)
 PASS  test/merge/defaults/merge-defaults.test.js (8.652s)
 PASS  test/help/help-multi-args.test.js (9.22s)
 PASS  test/env/prod/prod.test.js (10.27s)
 PASS  test/env/array/array-env.test.js (12.697s)
 PASS  test/entry/defaults-index/entry-multi-args.test.js (9.045s)
 PASS  test/source-map/array/source-map-array.test.js (9.321s)
 PASS  test/config-lookup/dotfolder-array/dotfolder-array.test.js (5.3s)
 PASS  test/typescript/typescript.test.js (12.304s)
 PASS  test/config/type/array/array-config.test.js (5.034s)
 PASS  test/env/object/object-env.test.js (9.816s)
 FAIL  test/config/basic/basic-config.test.js
  ● basic config file › is able to understand and parse a very basic configuration file

    expect(received).toBe(expected) // Object.is equality

    Expected: null
    Received: [Error: ENOENT: no such file or directory, stat '/home/travis/build/webpack/webpack-cli/test/config/basic/binary/a.bundle.js']

      10 |         expect(stdout).not.toBe(undefined);
      11 |         stat(resolve(__dirname, './binary/a.bundle.js'), (err, stats) => {
    > 12 |             expect(err).toBe(null);
         |                         ^
      13 |             expect(stats.isFile()).toBe(true);
      14 |             done();
      15 |         });

      at test/config/basic/basic-config.test.js:12:25

 PASS  test/config-lookup/dotfolder-single/dotfolder-single.test.js
 FAIL  test/entry/config-entry/entry-with-command/entry-with-command.test.js
  ● config entry and command entry all exist › should use command entry if config command existed

    expect(received).toContain(expected) // indexOf

    Expected substring: "./index.js"
    Received string:    "�[?25l? Which flags do you want to use? …·
    ✔ --entry: The entry point of your application.
    ✔ --config: Provide path to a webpack configuration file
    ✔ --merge: Merge a configuration file using webpack-merge
    ✔ --progress: Print compilation progress during build
    ✔ --silent: Disable any output that webpack makes
    ✔ --help: Outputs list of supported flags
    ✔ --defaults: Allow webpack to set defaults aggresively
    ✔ --output: Output location of the file generated by webpack
    ✔ --plugin: Load a given plugin
    ✔ --target: Sets the build target
    ✔ --watch: Watch for files changes
    ✔ --hot: Enables Hot Module Replacement
    ✔ --sourcemap: Determine source maps to use
    ✔ --prefetch: Prefetch this request
    ✔ --json: Prints result as JSON
    ✔ --dev: Run development build
    ✔ --prod: Run production build
    ✔ --mode: Defines the mode to pass to webpack
    ✔ --no-mode: Sets mode=\"none\" which disables any default behavior
    ✔ --version: Get current version
    ✔ --node-args: NodeJS flags
    ✔ --stats: It instructs webpack on how to treat the stats
    ✔ --verbose: It tells webpack to output all the information�[23A�[37G�[?25h"

       8 |         const { stdout } = run(__dirname, ['-c', '../1.js', './index.js'], false);
       9 | 
    > 10 |         expect(stdout).toContain('./index.js');
         |                        ^
      11 |         stat(resolve(__dirname, './binary/main.bundle.js'), (err, stats) => {
      12 |             expect(err).toBeFalsy();
      13 |             expect(stats.isFile()).toBe(true);

      at Object.<anonymous> (test/entry/config-entry/entry-with-command/entry-with-command.test.js:10:24)

 PASS  test/merge/config/merge-config.test.js
 PASS  test/version/version-single-arg.test.js (6.105s)
 PASS  test/defaults/without-config-and-entry/default-without-config.test.js
 PASS  test/entry/config-entry/entry-with-config/entry-with-config.test.js
 PASS  test/config/type/function/function-config.test.js
 FAIL  test/target/node/node-test.test.js
  ● Node target › should emit the correct code

    expect(received).toBeFalsy()

    Received: "[webpack-cli] Unknown argument: /home/travis/build/webpack/webpack-cli/test/target/node"

       7 |     it('should emit the correct code', done => {
       8 |         const { stderr } = run(__dirname, [__dirname, '-c', './webpack.config.js']);
    >  9 |         expect(stderr).toBeFalsy();
         |                        ^
      10 |         stat(resolve(__dirname, 'bin/main.js'), (err, stats) => {
      11 |             expect(err).toBe(null);
      12 |             expect(stats.isFile()).toBe(true);

      at Object.<anonymous> (test/target/node/node-test.test.js:9:24)

 PASS  test/standard/standard.test.js
 PASS  test/config/empty/empty-config.test.js
 PASS  test/entry/scss/scss.test.js (6.541s)
 PASS  test/entry/defaults-empty/entry-single-arg.test.js
 PASS  test/unknown/unknown.test.js
�[999D�[K
Summary of all failing tests
 FAIL  test/config/basic/basic-config.test.js
  ● basic config file › is able to understand and parse a very basic configuration file

    expect(received).toBe(expected) // Object.is equality

    Expected: null
    Received: [Error: ENOENT: no such file or directory, stat '/home/travis/build/webpack/webpack-cli/test/config/basic/binary/a.bundle.js']

      10 |         expect(stdout).not.toBe(undefined);
      11 |         stat(resolve(__dirname, './binary/a.bundle.js'), (err, stats) => {
    > 12 |             expect(err).toBe(null);
         |                         ^
      13 |             expect(stats.isFile()).toBe(true);
      14 |             done();
      15 |         });

      at test/config/basic/basic-config.test.js:12:25

 FAIL  test/entry/config-entry/entry-with-command/entry-with-command.test.js
  ● config entry and command entry all exist › should use command entry if config command existed

    expect(received).toContain(expected) // indexOf

    Expected substring: "./index.js"
    Received string:    "�[?25l? Which flags do you want to use? …·
    ✔ --entry: The entry point of your application.
    ✔ --config: Provide path to a webpack configuration file
    ✔ --merge: Merge a configuration file using webpack-merge
    ✔ --progress: Print compilation progress during build
    ✔ --silent: Disable any output that webpack makes
    ✔ --help: Outputs list of supported flags
    ✔ --defaults: Allow webpack to set defaults aggresively
    ✔ --output: Output location of the file generated by webpack
    ✔ --plugin: Load a given plugin
    ✔ --target: Sets the build target
    ✔ --watch: Watch for files changes
    ✔ --hot: Enables Hot Module Replacement
    ✔ --sourcemap: Determine source maps to use
    ✔ --prefetch: Prefetch this request
    ✔ --json: Prints result as JSON
    ✔ --dev: Run development build
    ✔ --prod: Run production build
    ✔ --mode: Defines the mode to pass to webpack
    ✔ --no-mode: Sets mode=\"none\" which disables any default behavior
    ✔ --version: Get current version
    ✔ --node-args: NodeJS flags
    ✔ --stats: It instructs webpack on how to treat the stats
    ✔ --verbose: It tells webpack to output all the information�[23A�[37G�[?25h"

       8 |         const { stdout } = run(__dirname, ['-c', '../1.js', './index.js'], false);
       9 | 
    > 10 |         expect(stdout).toContain('./index.js');
         |                        ^
      11 |         stat(resolve(__dirname, './binary/main.bundle.js'), (err, stats) => {
      12 |             expect(err).toBeFalsy();
      13 |             expect(stats.isFile()).toBe(true);

      at Object.<anonymous> (test/entry/config-entry/entry-with-command/entry-with-command.test.js:10:24)

 FAIL  test/target/node/node-test.test.js
  ● Node target › should emit the correct code

    expect(received).toBeFalsy()

    Received: "[webpack-cli] Unknown argument: /home/travis/build/webpack/webpack-cli/test/target/node"

       7 |     it('should emit the correct code', done => {
       8 |         const { stderr } = run(__dirname, [__dirname, '-c', './webpack.config.js']);
    >  9 |         expect(stderr).toBeFalsy();
         |                        ^
      10 |         stat(resolve(__dirname, 'bin/main.js'), (err, stats) => {
      11 |             expect(err).toBe(null);
      12 |             expect(stats.isFile()).toBe(true);

      at Object.<anonymous> (test/target/node/node-test.test.js:9:24)


Test Suites: 3 failed, 3 skipped, 38 passed, 41 of 44 total
Tests:       3 failed, 3 skipped, 104 passed, 110 total
Snapshots:   0 total
Time:        117.052s
Ran all test suites matching /test\//i.
�[2K�[1Gerror Command failed with exit code 1.
�[2K�[1Ginfo Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
�[2K�[1Gerror Command failed with exit code 1.
�[2K�[1Ginfo Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
The command "yarn test:ci" exited with 1.
store build cache [...]

Done. Your build exited with 1.

See complete report here.

@anshumanv
Copy link
Member

How's the progress on this? @rishabh3112

@alexander-akait
Copy link
Member

@anshumanv @rishabh3112 need help, I want to look at that on the next week, but feel free to go ahead

@rishabh3112
Copy link
Member Author

I couldn't resolve the 3 tests which are failing. Need help in that.

@snitin315
Copy link
Member

I couldn't resolve the 3 tests which are failing. Need help in that.

I guess #1455 should fix node-test for you.

node

@knagaitsev knagaitsev mentioned this pull request Apr 18, 2020
@anshumanv
Copy link
Member

Closing in favour of #1481

@anshumanv anshumanv closed this Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants