Fix migrate command #1495

knagaitsev · 2020-04-23T22:21:40Z

What kind of change does this PR introduce?

fix/tests

Did you add tests for your changes?

Yes (still need to add tests for runPrettier)

I'm not exactly sure how the migrate command is supposed to work. Could someone give an example of a webpack config that it should make modifications to?

If relevant, did you update the documentation?

No

Summary

@jamesgeorge007 Sorry for taking over, just couldn't debug runPromptWithAnswers without using your changes. It seems that execa does not like an empty input string option.

Fixes runPromptWithAnswers
Fixes migrate
Fixes issues with runPrettier

Does this PR introduce a breaking change?

Slightly with how runPrettier works

Other information

anshumanv · 2020-04-24T04:12:30Z

@Loonride one test unusually still fails, doesn't show any log, can you debug?

jamesgeorge007 · 2020-04-24T04:12:47Z

I'm not exactly sure how the migrate command is supposed to work. Could someone give an example of a webpack config that it should make modifications to?

You can find everything related to migrate command here

jamesgeorge007 · 2020-04-24T04:22:05Z

test/utils/test-utils.js

-    });
+    };
+
+    // some tests don't work if the input option is an empty string


Good catch 👏

knagaitsev · 2020-04-24T21:41:23Z

It seems sending a SIGKILL to the process at the end of runPromptWithAnswers made it more stable 🤷‍♀️

alexander-akait

/cc @webpack/cli-team please review

alexander-akait · 2020-04-25T15:19:28Z

hm, ci is not stable in some cases, it is strange

knagaitsev · 2020-04-25T15:31:51Z

hm, ci is not stable in some cases, it is strange

whats confusing is it always either seems like a large group of them fail, or none of them fail 🤔

rishabh3112 · 2020-04-25T15:33:54Z

whats confusing is it always either seems like a large group of them fail, or none of them fail 🤔

Others are cancelled when one of them fails. the macOS Node v13 container is the failing one in every PR:(

knagaitsev · 2020-04-25T15:34:32Z

Others are cancelled when one of them fails. the macOS Node v13 container is the failing one everywhere :(

Oh that makes more sense thanks

alexander-akait · 2020-04-25T15:48:22Z

I think we have racing

webpack-bot · 2020-04-25T16:00:40Z

@rishabh3112 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@jamesgeorge007 Please review the new changes.

knagaitsev · 2020-04-25T16:11:58Z

Let me try a few things with how runPromptWithAnswers works.

…on runWatch

knagaitsev · 2020-04-25T18:10:34Z

@evilebottnawi 4 test runs good in a row, I think stabilization worked!

What I did was forced runPromptWithAnswers to wait for new stdout before writing the next answer to stdin

alexander-akait · 2020-04-27T11:43:30Z

Good job!

knagaitsev requested a review from a team as a code owner April 23, 2020 22:21

webpack-bot added the PR: unreviewed label Apr 23, 2020

knagaitsev force-pushed the hotfix/migrate branch from 1942ef2 to ca0eef5 Compare April 23, 2020 22:42

jamesgeorge007 reviewed Apr 24, 2020

View reviewed changes

test/utils/test-utils.js Outdated

});

};

// some tests don't work if the input option is an empty string

Copy link

Member

jamesgeorge007 Apr 24, 2020

Choose a reason for hiding this comment

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

Good catch 👏

alexander-akait mentioned this pull request Apr 25, 2020

features: make cli help interface more documented #1467

Closed

alexander-akait previously approved these changes Apr 25, 2020

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: unreviewed labels Apr 25, 2020

anshumanv previously approved these changes Apr 25, 2020

View reviewed changes

jamesgeorge007 previously approved these changes Apr 25, 2020

View reviewed changes

webpack-bot added PR: review-outdated and removed PR: reviewed-approved labels Apr 25, 2020

jamesgeorge007 and others added 8 commits April 25, 2020 11:15

fix: regression with migrate command

d68f085

chore: add integration tests for migrate

a7059db

tests: check for generated config file

2d1a6b1

chore: fix runPromptWithAnswers and migrate prettier

d950513

tests: added run prettier tests

12396ad

tests: updated migrate test to check output result

d5bbfd6

chore: stabilize output test

1931f12

chore: revert output test changes

f16c2a8

knagaitsev added 3 commits April 25, 2020 11:17

chore: check stability without migrate prompt tests

7215334

chore: add stability to runPromptWithAnswers by killing at end

b40b6e8

chore: reset output test

2b8a43d

knagaitsev dismissed stale reviews from jamesgeorge007, anshumanv, and alexander-akait via 2b8a43d April 25, 2020 16:23

knagaitsev force-pushed the hotfix/migrate branch from e29d4db to 2b8a43d Compare April 25, 2020 16:23

webpack-bot added the PR: unreviewed label Apr 25, 2020

knagaitsev added 4 commits April 25, 2020 11:56

chore: make runPromptWithAnswers wait for input

4350bd7

chore: update docs for runPromptWithAnswers

987e50e

chore: add comment explaining timeout in runPromptWithAnswers

8350dc5

chore: make output wait default for runPromptWithAnswers, force pipe …

11aec58

…on runWatch

anshumanv approved these changes Apr 26, 2020

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated PR: unreviewed labels Apr 26, 2020

anshumanv requested a review from alexander-akait April 27, 2020 11:39

alexander-akait approved these changes Apr 27, 2020

View reviewed changes

alexander-akait merged commit 7ebcbb8 into webpack:next Apr 27, 2020

This was referenced Apr 27, 2020

Migrate functionality doesn't work as expected #1422

Closed

fix: regression with migrate command #1423

Closed

jamesgeorge007 linked an issue Apr 27, 2020 that may be closed by this pull request

Migrate functionality doesn't work as expected #1422

Closed

enterstudio mentioned this pull request Jun 20, 2022

[Snyk] Security upgrade got from 6.7.1 to 11.8.5 enterstudio/webpack-cli#24

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix migrate command #1495

Fix migrate command #1495

knagaitsev commented Apr 23, 2020

anshumanv commented Apr 24, 2020

jamesgeorge007 commented Apr 24, 2020

jamesgeorge007 Apr 24, 2020

knagaitsev commented Apr 24, 2020

alexander-akait left a comment

alexander-akait commented Apr 25, 2020

knagaitsev commented Apr 25, 2020

rishabh3112 commented Apr 25, 2020 •

edited

Loading

knagaitsev commented Apr 25, 2020

alexander-akait commented Apr 25, 2020

webpack-bot commented Apr 25, 2020

knagaitsev commented Apr 25, 2020

knagaitsev commented Apr 25, 2020 •

edited

Loading

alexander-akait commented Apr 27, 2020

Fix migrate command #1495

Fix migrate command #1495

Conversation

knagaitsev commented Apr 23, 2020

anshumanv commented Apr 24, 2020

jamesgeorge007 commented Apr 24, 2020

jamesgeorge007 Apr 24, 2020

Choose a reason for hiding this comment

knagaitsev commented Apr 24, 2020

alexander-akait left a comment

Choose a reason for hiding this comment

alexander-akait commented Apr 25, 2020

knagaitsev commented Apr 25, 2020

rishabh3112 commented Apr 25, 2020 • edited Loading

knagaitsev commented Apr 25, 2020

alexander-akait commented Apr 25, 2020

webpack-bot commented Apr 25, 2020

knagaitsev commented Apr 25, 2020

knagaitsev commented Apr 25, 2020 • edited Loading

alexander-akait commented Apr 27, 2020

rishabh3112 commented Apr 25, 2020 •

edited

Loading

knagaitsev commented Apr 25, 2020 •

edited

Loading