tests(utils): move tests to TS #688

hemal7735 · 2018-11-08T02:39:58Z

What kind of change does this PR introduce?

We are moving utils package test-cases to TS.

Did you add tests for your changes? NA

If relevant, did you update the documentation? NA

Summary

This PR address the #613 - tests(migration): typescript

Does this PR introduce a breaking change? NA

Other information

There are some tests files for which help is needed, I have changed them to `.tx` to allow me to push to GitHub.

Pending Tasks:

npm-packages-exists.test.tx - TS mock issue - npm-exists module is not getting mocked, as actual call is going
recursive-parser.test.tx -- defineTest parameter issue , already mentioned in tests(migration): typescript #613
cleanup equivalent .js , __fixtures__ and __snapshots__ files. (these are not removed for reference)

hemal7735 · 2018-11-08T02:41:44Z

@ematipico can you help moving things forwards?

hemal7735 · 2018-11-09T06:47:49Z

@dhruvdutt can you help me with the blockers?

evenstensberg

ping ping @webpack/cli-team

ematipico · 2018-11-11T06:54:55Z

Could please some comment on your code describing the problem in a detailed way?

ematipico

Left some comments

packages/utils/__tests__/npm-packages-exists.test.tx

packages/utils/__tests__/recursive-parser.test.tx

webpack-bot · 2018-11-12T05:56:35Z

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

hemal7735 · 2018-11-13T09:28:27Z

@evenstensberg @ematipico @dhruvdutt please review this as all pending tasks are completed now.

evenstensberg · 2018-11-15T02:48:42Z

packages/utils/__tests__/ast-utils.test.ts

@@ -105,41 +105,41 @@ const a = { plugs: [] }
 	describe("createOrUpdatePluginByName", () => {
 		it("should create a new plugin without arguments", () => {
 			const ast = j("{ plugins: [] }");
-			ast.find(j.ArrayExpression).forEach(node => {
-				utils.createOrUpdatePluginByName(j, node, "Plugin");
+			ast.find(j.ArrayExpression).forEach((node) => {


@ematipico I think we were agreeing on non-enclosed paran when single arg?

Current tslint configuration is strict in those areas. eslint config is still relaxed.

Can we make it as we agreed? Think Emanuelle has some comments there

We will need to use "arrow-parens": [true, "ban-single-arg-parens"]" in tslint config
more info: https://palantir.github.io/tslint/rules/arrow-parens/

~~forEach((node) => {~~ forEach((node: INode) => {

@hemal7735 Can you follow this for all occurrences of node and path. Check ast-utils.ts files for reference.

dhruvdutt

Sorry for the late response. 😅
Minor changes but looks good overall. 💯

packages/utils/__tests__/npm-packages-exists.test.ts

packages/utils/__tests__/ast-utils.test.ts

packages/utils/__tests__/__snapshots__/ast-utils.test.ts.snap

evenstensberg

Ts thing on param needs to be sorted out

hemal7735 · 2018-11-16T18:06:43Z

Ts thing on param needs to be sorted out

@evenstensberg are you talking about parenthesis from previous conversation?

dhruvdutt

Add INode type for all node paths.

hemal7735 · 2018-11-16T19:15:46Z

Add INode type for all node paths.

@dhruvdutt can you comment on the file where you think change is required?

webpack-bot · 2018-11-17T05:59:59Z

@hemal7735 Thanks for your update.

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

@dhruvdutt Please review the new changes.

ematipico · 2018-11-17T06:50:36Z

Resolving the TS linter problem will cause a waterfall effect where other files will be affected by the change. We can make a separate PR to fix the linter and we keep focusing on migrating the tests

dhruvdutt · 2018-11-17T08:08:28Z

Agreed. I'm working on removing tslint. We can merge this for now.

evenstensberg

Lgtm

dhruvdutt · 2018-11-17T19:46:37Z

Please fix conflicts.

hemal7735 · 2018-11-17T19:49:34Z

Please fix conflicts.

@dhruvdutt I don't see github showing me any conflict.

* chore(test): add migrate test * chore(lint): revert change * chore(test): moved to typescript migrate tests * tests(ts): moved module concatenation plugin to ts * tests(migrate): moved to typescript * tests(webpack-scaffold): moved to typescript * tests(utils): copy fixtures to __tests_ * tests(utils): move package-manager.test.js to TS * tests(utils): move validate-identifier.test.js to TS * tests(utils): move resolve-packages.test.js to TS * tests(utils): is-local-path.test.js to TS * tests(utils): move npm-exists.test.js to TS * tests(utils): move ast-utils.test.js to TS * tests(utils): WIP-test-cases * tests(utils): move recursive-parser.test.js to TS * tests(utils): move npm-package-exists.test.js to TS * tests(utils): cleanup .js , __fixtures__ and __snapshots__ files * chore(utils): clean console recursive-parser.test.ts * chore(utils): make options param optional * chore: use INode in ast-utils.test.ts

* chore(test): add migrate test * chore(lint): revert change * chore(test): moved to typescript migrate tests * tests(ts): moved module concatenation plugin to ts * tests(migrate): moved to typescript * tests(webpack-scaffold): moved to typescript * tests(migrate): clean js text fixtures * tests(migrate): update babel-loader test * tests(migrate): update babel-loader snapshot * tests(utils): move tests to TS (#688) * chore(test): add migrate test * chore(lint): revert change * chore(test): moved to typescript migrate tests * tests(ts): moved module concatenation plugin to ts * tests(migrate): moved to typescript * tests(webpack-scaffold): moved to typescript * tests(utils): copy fixtures to __tests_ * tests(utils): move package-manager.test.js to TS * tests(utils): move validate-identifier.test.js to TS * tests(utils): move resolve-packages.test.js to TS * tests(utils): is-local-path.test.js to TS * tests(utils): move npm-exists.test.js to TS * tests(utils): move ast-utils.test.js to TS * tests(utils): WIP-test-cases * tests(utils): move recursive-parser.test.js to TS * tests(utils): move npm-package-exists.test.js to TS * tests(utils): cleanup .js , __fixtures__ and __snapshots__ files * chore(utils): clean console recursive-parser.test.ts * chore(utils): make options param optional * chore: use INode in ast-utils.test.ts * tests(migrate): adds interface ILazyTransformObject * chore(commitlint): remove deprecated lang rule * chore(typings): fix tslint error * tests(migration): refactor dirName in a variable * tests(migrate): refactor dirName in variable 2 * tests(migrate): import path.resolve & add rootPath variable * tests(utils): adds type in forEach * tests(migrate): refactor resolve.test * chore(tests): enable ts in tests * chore(tests): remove unwanted script

sendilkumarn and others added 14 commits November 4, 2018 15:17

chore(test): add migrate test

0a590a0

chore(lint): revert change

dc79068

chore(test): moved to typescript migrate tests

dd3a76c

tests(ts): moved module concatenation plugin to ts

aa80327

tests(migrate): moved to typescript

5a34a88

tests(webpack-scaffold): moved to typescript

122a770

tests(utils): copy fixtures to __tests_

3a9656a

tests(utils): move package-manager.test.js to TS

a3c9e32

tests(utils): move validate-identifier.test.js to TS

bb7e3ed

tests(utils): move resolve-packages.test.js to TS

655715d

tests(utils): is-local-path.test.js to TS

f470f5f

tests(utils): move npm-exists.test.js to TS

182f0bb

tests(utils): move ast-utils.test.js to TS

86b5d7d

tests(utils): WIP-test-cases

c2c1479

webpack-bot added the PR: unreviewed label Nov 8, 2018

evenstensberg reviewed Nov 10, 2018

View reviewed changes

ematipico reviewed Nov 11, 2018

View reviewed changes

packages/utils/__tests__/npm-packages-exists.test.tx Outdated Show resolved Hide resolved

packages/utils/__tests__/recursive-parser.test.tx Outdated Show resolved Hide resolved

hemal7735 added 3 commits November 12, 2018 08:37

tests(utils): move recursive-parser.test.js to TS

b351fa2

tests(utils): move npm-package-exists.test.js to TS

7bda1ff

tests(utils): cleanup .js , __fixtures__ and __snapshots__ files

1c8d42a

hemal7735 changed the title ~~tests(utils): WIP move tests to TS~~ tests(utils): move tests to TS Nov 12, 2018

webpack-bot added the PR: CI-ok label Nov 12, 2018

evenstensberg reviewed Nov 15, 2018

View reviewed changes

dhruvdutt reviewed Nov 16, 2018

View reviewed changes

packages/utils/__tests__/npm-packages-exists.test.ts Outdated Show resolved Hide resolved

packages/utils/__tests__/ast-utils.test.ts Outdated Show resolved Hide resolved

packages/utils/__tests__/__snapshots__/ast-utils.test.ts.snap Outdated Show resolved Hide resolved

dhruvdutt force-pushed the ts-test branch from 122a770 to 4c14ccf Compare November 16, 2018 14:52

webpack-bot added PR: conflict and removed PR: CI-ok labels Nov 16, 2018

Merge remote-tracking branch 'GRO/ts-test' into ts-test-utils

aec65b8

webpack-bot removed the PR: conflict label Nov 16, 2018

dhruvdutt approved these changes Nov 16, 2018

View reviewed changes

webpack-bot added PR: reviewed-approved PR: CI-ok and removed PR: unreviewed labels Nov 16, 2018

evenstensberg requested changes Nov 16, 2018

View reviewed changes

webpack-bot added the PR: reviewed-changes-requested label Nov 16, 2018

dhruvdutt requested changes Nov 16, 2018

View reviewed changes

chore: use INode in ast-utils.test.ts

8af2a8b

webpack-bot added PR: review-outdated and removed PR: reviewed-approved PR: reviewed-changes-requested labels Nov 17, 2018

ematipico approved these changes Nov 17, 2018

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels Nov 17, 2018

dhruvdutt approved these changes Nov 17, 2018

View reviewed changes

evenstensberg approved these changes Nov 17, 2018

View reviewed changes

ematipico merged commit 243ea58 into webpack:ts-test Nov 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests(utils): move tests to TS #688

tests(utils): move tests to TS #688

hemal7735 commented Nov 8, 2018 •

edited

hemal7735 commented Nov 8, 2018

hemal7735 commented Nov 9, 2018

evenstensberg left a comment

ematipico commented Nov 11, 2018

ematipico left a comment

webpack-bot commented Nov 12, 2018

hemal7735 commented Nov 13, 2018

evenstensberg Nov 15, 2018

hemal7735 Nov 15, 2018

evenstensberg Nov 15, 2018

hemal7735 Nov 15, 2018

dhruvdutt Nov 17, 2018 •

edited

dhruvdutt left a comment

evenstensberg left a comment

hemal7735 commented Nov 16, 2018

dhruvdutt left a comment

hemal7735 commented Nov 16, 2018

webpack-bot commented Nov 17, 2018

ematipico commented Nov 17, 2018

dhruvdutt commented Nov 17, 2018

evenstensberg left a comment

dhruvdutt commented Nov 17, 2018

hemal7735 commented Nov 17, 2018

tests(utils): move tests to TS #688

tests(utils): move tests to TS #688

Conversation

hemal7735 commented Nov 8, 2018 • edited

Other information

There are some tests files for which help is needed, I have changed them to .tx to allow me to push to GitHub.

hemal7735 commented Nov 8, 2018

hemal7735 commented Nov 9, 2018

evenstensberg left a comment

Choose a reason for hiding this comment

ematipico commented Nov 11, 2018

ematipico left a comment

Choose a reason for hiding this comment

webpack-bot commented Nov 12, 2018

hemal7735 commented Nov 13, 2018

evenstensberg Nov 15, 2018

Choose a reason for hiding this comment

hemal7735 Nov 15, 2018

Choose a reason for hiding this comment

evenstensberg Nov 15, 2018

Choose a reason for hiding this comment

hemal7735 Nov 15, 2018

Choose a reason for hiding this comment

dhruvdutt Nov 17, 2018 • edited

Choose a reason for hiding this comment

dhruvdutt left a comment

Choose a reason for hiding this comment

evenstensberg left a comment

Choose a reason for hiding this comment

hemal7735 commented Nov 16, 2018

dhruvdutt left a comment

Choose a reason for hiding this comment

hemal7735 commented Nov 16, 2018

webpack-bot commented Nov 17, 2018

ematipico commented Nov 17, 2018

dhruvdutt commented Nov 17, 2018

evenstensberg left a comment

Choose a reason for hiding this comment

dhruvdutt commented Nov 17, 2018

hemal7735 commented Nov 17, 2018

hemal7735 commented Nov 8, 2018 •

edited

There are some tests files for which help is needed, I have changed them to `.tx` to allow me to push to GitHub.

dhruvdutt Nov 17, 2018 •

edited