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

chore: create package run (WIP) #1308

Closed
wants to merge 10 commits into from

Conversation

rishabh3112
Copy link
Member

What kind of change does this PR introduce?
refactor, Moves executer to it's own package

Did you add tests for your changes?
Will be moving tests to the repo in this PR itself, for now there is a dummy test.

If relevant, did you update the documentation?
No, will write documentation for the package in this PR itself.

Summary
As suggested by Emanuele, I am moving the executer to it's seperate package.

Does this PR introduce a breaking change?
No

Other information
Tasks needed to be done here

  • Move Tests
  • Write documentation
  • Release package to npm

@rishabh3112 rishabh3112 changed the title chore: create package run chore: create package run (WIP) Mar 10, 2020
@rishabh3112
Copy link
Member Author

@ematipico should I keep the new package as peer or direct dependency to the webpack-cli package ?

@webpack-bot
Copy link

@rishabh3112 Please review the following output log for errors:

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=integration


Setting up build cache [...]
 [...]
$ node --version
v10.19.0
$ 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/logger /home/travis/build/webpack/webpack-cli/packages/migrate /home/travis/build/webpack/webpack-cli/packages/package-utils /home/travis/build/webpack/webpack-cli/packages/run /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

/home/travis/build/webpack/webpack-cli/packages/package-utils/__tests__/index.test.ts
  10:24  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  16:29  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

✖ 2 problems (0 errors, 2 warnings)

�[2K�[1GDone in 15.73s.
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 (13.779s)
 PASS  test/no-mode/no-mode.test.js (26.91s)
 PASS  test/utils/test-utils.test.js
 PASS  test/output/named-bundles/output-named-bundles.test.js (20.705s)
 PASS  test/mode/prod/prod.test.js (38.142s)
 PASS  test/global/global.test.js (43.871s)
 PASS  test/info/info-help.test.js (18.578s)
 PASS  test/mode/dev/dev.test.js (16.855s)
 PASS  test/help/help-single-arg.test.js (14.71s)
 PASS  test/version/version-multi-args.test.js (17.966s)
 PASS  test/serve/serve-basic.test.js (26.607s)
 PASS  test/defaults/output-defaults.test.js (17.962s)
 PASS  test/stats/stats.test.js (46.048s)
 PASS  test/info/info-output.test.js (25.224s)
 PASS  test/help/help-flags.test.js (11.187s)
 PASS  test/source-map/object/source-map-object.test.js (17.056s)
 PASS  test/help/help-commands.test.js (10.836s)
 PASS  test/info/info-multi-args.test.js (21.948s)
 PASS  test/merge/defaults/merge-defaults.test.js (10.432s)
 PASS  test/help/help-multi-args.test.js (10.739s)
 PASS  test/entry/defaults-index/entry-multi-args.test.js (9.962s)
 PASS  test/env/prod/prod.test.js (12.192s)
 FAIL  test/entry/config-entry/entry-with-command/entry-with-command.test.js (5.07s)
  ● config entry and command entry all exist › should use command entry if config command existed

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

    Matcher error: received value must not be null nor undefined

    Received has value: undefined

       9 |         const summary = extractSummary(stdout);
      10 |         const outputDir = 'entry-with-command/binary';
    > 11 |         expect(summary['Output Directory']).toContain(outputDir);
         |                                             ^
      12 | 
      13 |         expect(stdout).toContain('./index.js');
      14 |         stat(resolve(__dirname, './binary/main.bundle.js'), (err, stats) => {

      at Object.done (test/entry/config-entry/entry-with-command/entry-with-command.test.js:11:45)

 PASS  test/source-map/array/source-map-array.test.js (10.869s)
 PASS  test/env/array/array-env.test.js (15.535s)
 PASS  test/typescript/typescript.test.js (12.663s)
 PASS  test/config-lookup/dotfolder-array/dotfolder-array.test.js (5.801s)
 PASS  test/env/object/object-env.test.js (11.234s)
 PASS  test/config/basic/basic-config.test.js (5.456s)
 FAIL  test/entry/config-entry/entry-with-config/entry-with-config.test.js (5.61s)
  ● default entry and config entry all exist › should use config entry if config entry existed

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

    Matcher error: received value must not be null nor undefined

    Received has value: undefined

       9 |         const summary = extractSummary(stdout);
      10 |         const outputDir = 'entry-with-config/binary';
    > 11 |         expect(summary['Output Directory']).toContain(outputDir);
         |                                             ^
      12 | 
      13 |         expect(stdout).toContain('./a.js');
      14 |         stat(resolve(__dirname, './binary/index.bundle.js'), (err, stats) => {

      at Object.done (test/entry/config-entry/entry-with-config/entry-with-config.test.js:11:45)

 PASS  test/config/type/array/array-config.test.js (5.368s)
 PASS  test/config-lookup/dotfolder-single/dotfolder-single.test.js (5.471s)
 PASS  test/merge/config/merge-config.test.js (5.079s)
 PASS  test/defaults/without-config-and-entry/default-without-config.test.js (5.275s)
 PASS  test/version/version-single-arg.test.js (7.058s)
 PASS  test/config/type/function/function-config.test.js (5.496s)
 PASS  test/target/node/node-test.test.js
 PASS  test/output/pretty/pretty.test.js (5.228s)
 PASS  test/entry/scss/scss.test.js (7.726s)
 PASS  test/standard/standard.test.js (5.752s)
 PASS  test/config/empty/empty-config.test.js (5.029s)
 PASS  test/unknown/unknown.test.js
 PASS  test/entry/defaults-empty/entry-single-arg.test.js
�[999D�[K
Summary of all failing tests
 FAIL  test/entry/config-entry/entry-with-command/entry-with-command.test.js (5.07s)
  ● config entry and command entry all exist › should use command entry if config command existed

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

    Matcher error: received value must not be null nor undefined

    Received has value: undefined

       9 |         const summary = extractSummary(stdout);
      10 |         const outputDir = 'entry-with-command/binary';
    > 11 |         expect(summary['Output Directory']).toContain(outputDir);
         |                                             ^
      12 | 
      13 |         expect(stdout).toContain('./index.js');
      14 |         stat(resolve(__dirname, './binary/main.bundle.js'), (err, stats) => {

      at Object.done (test/entry/config-entry/entry-with-command/entry-with-command.test.js:11:45)

 FAIL  test/entry/config-entry/entry-with-config/entry-with-config.test.js (5.61s)
  ● default entry and config entry all exist › should use config entry if config entry existed

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

    Matcher error: received value must not be null nor undefined

    Received has value: undefined

       9 |         const summary = extractSummary(stdout);
      10 |         const outputDir = 'entry-with-config/binary';
    > 11 |         expect(summary['Output Directory']).toContain(outputDir);
         |                                             ^
      12 | 
      13 |         expect(stdout).toContain('./a.js');
      14 |         stat(resolve(__dirname, './binary/index.bundle.js'), (err, stats) => {

      at Object.done (test/entry/config-entry/entry-with-config/entry-with-config.test.js:11:45)


Test Suites: 2 failed, 3 skipped, 41 passed, 43 of 46 total
Tests:       2 failed, 3 skipped, 111 passed, 116 total
Snapshots:   0 total
Time:        146.709s
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.

@ematipico
Copy link
Contributor

I don't know yet. I still have to understand how's the whole user flow.

@alexander-akait
Copy link
Member

alexander-akait commented Mar 11, 2020

Why we need that package? What is use case? How it can be used stadalone? As I see it makes no sense to use this package without webpack-cli. Therefore, separate it into two packages makes no sense

@rishabh3112
Copy link
Member Author

This was suggested by @ematipico to separate it from main package to reduce dependency of main package.

For more context, you may look at #1261 and the PR where I implemented it.

@rishabh3112
Copy link
Member Author

I think about it overall and shifting it to it's own package will not be helpful in reducing main package size. So, I am closing this PR.
Thanks everyone!

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.

4 participants