FoalTS / foal

Full-featured Node.js framework, with no complexity. 🚀 Simple and easy to use, TypeScript-based and well-documented.
https://foalts.org/
MIT License
1.9k stars 140 forks source link

Encoding problems when running the `cli` tests on Windows #207

Closed LoicPoullain closed 5 years ago

LoicPoullain commented 6 years ago

Issue

Issue reported in the PR https://github.com/FoalTS/foal/pull/206. When running the tests of the cli package on Windows, they all seem to fail because of a line break encoding (\n vs \r\n).

Possible solutions

OR

Jrfidellis commented 6 years ago

After updating the following line: strictEqual(actual, spec); to strictEqual(actual.replace(/\r\n/g, '\n'), spec.replace(/\r\n/g, '\n')); the number of errors was reduced to 5:

 1) createController when the directory src/app/controllers exists and if register is true should update the "controllers = [ \n \n ]" property in src/app/app.module.ts if it exists.:

      AssertionError [ERR_ASSERTION]: '// 3p\nimport {} from \'somewhere\';\nimport { TestFooBarController } from \'./controllers\';\n\nexport class MyModule {\n  con === '// 3p\nimport {} from \'somewhere\';\nimport { TestFooBarController } from \'./controllers\';\n\nexport class MyModule {\n  con
      + expected - actual

       import { TestFooBarController } from './controllers';

       export class MyModule {
         controllers = [
      -
      +    controller('/', TestFooBarController)
         ];
       }

      at TestEnvironment.validateSpec (src\generate\utils\test-environment.ts:51:5)
      at Context.it (src\generate\generators\controller\create-controller.spec.ts:135:10)

  2) createController when the directory src/app/controllers exists and if register is true should update the "controllers = [ \n (.*) \n ]" property in src/app/app.module.ts if it exists.:

      AssertionError [ERR_ASSERTION]: '// 3p\nimport { controller } from \'@foal/core\';\nimport { TestFooBarController } from \'./controllers\';\n\nexport class MyMo === '// 3p\nimport { controller } from \'@foal/core\';\nimport { TestFooBarController } from \'./controllers\';\n\nexport class MyMo
      + expected - actual

       export class MyModule {
         controllers = [
           controller('/', MyController),
      -    controller('/', MyController2)
      +    controller('/', MyController2),
      +    controller('/', TestFooBarController)
         ];
       }

      at TestEnvironment.validateSpec (src\generate\utils\test-environment.ts:51:5)
      at Context.it (src\generate\generators\controller\create-controller.spec.ts:144:10)

  3) createModule should render the root templates in src/app/sub-modules/ if the directory exists.:

      AssertionError [ERR_ASSERTION]: 'export { BarFooModule } from \'./bar-foo\';\nexport { TestFooBarModule } from \'./test-foo-bar\';\n' === 'export { BarFooModule } from \'./bar-foo\';\r\nexport { TestFooBarModule } from \'./test-foo-bar\';\r\n'
      + expected - actual

      -export { BarFooModule } from './bar-foo';
      -export { TestFooBarModule } from './test-foo-bar';
      +export { BarFooModule } from './bar-foo';
      +export { TestFooBarModule } from './test-foo-bar';

      at test (src\generate\generators\module\create-module.spec.ts:76:7)
      at Context.it (src\generate\generators\module\create-module.spec.ts:81:7)

  4) createModule should render the root templates in sub-modules/ if the directory exists.:

      AssertionError [ERR_ASSERTION]: 'export { BarFooModule } from \'./bar-foo\';\nexport { TestFooBarModule } from \'./test-foo-bar\';\n' === 'export { BarFooModule } from \'./bar-foo\';\r\nexport { TestFooBarModule } from \'./test-foo-bar\';\r\n'
      + expected - actual

      -export { BarFooModule } from './bar-foo';
      -export { TestFooBarModule } from './test-foo-bar';
      +export { BarFooModule } from './bar-foo';
      +export { TestFooBarModule } from './test-foo-bar';

      at test (src\generate\generators\module\create-module.spec.ts:76:7)
      at Context.it (src\generate\generators\module\create-module.spec.ts:86:7)

  5) createModule should render the root templates in the current directory otherwise.:

      AssertionError [ERR_ASSERTION]: 'export { BarFooModule } from \'./bar-foo\';\nexport { TestFooBarModule } from \'./test-foo-bar\';\n' === 'export { BarFooModule } from \'./bar-foo\';\r\nexport { TestFooBarModule } from \'./test-foo-bar\';\r\n'
      + expected - actual

      -export { BarFooModule } from './bar-foo';
      -export { TestFooBarModule } from './test-foo-bar';
      +export { BarFooModule } from './bar-foo';
      +export { TestFooBarModule } from './test-foo-bar';

      at test (src\generate\generators\module\create-module.spec.ts:76:7)
      at Context.it (src\generate\generators\module\create-module.spec.ts:90:7)
LoicPoullain commented 6 years ago

@Jrfidellis When you create a new project and run foal generate controller hello-world --register, is the property controllers of the app.module.ts file updated? I mean, is a new item controller('/', HelloWorldController) added?

LoicPoullain commented 6 years ago

If it is, then there is probably no changes to do with the code. I guess that the issue comes from how git clones the repository to your host (Windows). It may automatically convert the line endings LF (\n) to CRLF (\r\n) causing the tests to fail.

I created a new branch cli-test-encoding including a .gitattributes file to fix this problem. Could you git clone ... againt the repo, check out the branch and check if the tests succeed? Thanks!

Resources

Jrfidellis commented 6 years ago

It doesn't seems to work 😞

PS: the only change i did in the code was replacing the NODE_ENV=test to set NODE_ENV=test && at package.json

LoicPoullain commented 6 years ago

Can you open a file from your new clone and check the End of Line Sequence encoding? It is displayed in the bottom-right corner in vscode.

capture_d ecran_2018-09-14_a_06_49_24

Jrfidellis commented 6 years ago

image

LoicPoullain commented 6 years ago

Hmm it's weird, I just tried on Windows and it is working fine for me. It is maybe my fault, I told you to pull the branch and directly run the tests. Replacing NODE_ENV=test with set NODE_ENV=test&& is correct. Did you also add the line strictEqual(actual.replace(/\r\n/g, '\n'), spec.replace(/\r\n/g, '\n')); as well after pulling this branch?

Note: I'm not sure that spec.replace(/\r\n/g, '\n') is also required, is it?

Jrfidellis commented 5 years ago

I tested it again and it's working properly, thank you. I did replace the NODE_ENV=test with set NODE_ENV=test && and add the line strictEqual(actual.replace(/\r\n/g, '\n'), spec.replace(/\r\n/g, '\n')); (test-environment.ts file).

I tested without spec.replace(/\r\n/g, '\n'), but the tests failed.

LoicPoullain commented 5 years ago

Great! 💃 Do you want to submit a PR on that? The new branch should be created from cli-test-encoding to be sure that we have the proper encoding.

Regarding the environment variable problem, a workaround to make it both work on Windows and Unix systems is to use the --file flag of mocha. It lets us execute a file before all the tests which is useful here to change the env variable:

Jrfidellis commented 5 years ago

Hi @LoicPoullain,

I opened a pull request with the changes that you proposed. The Travis build isn't passing because of a missing newline in the 'test.ts' file.

Unfortunately i had to delete my fork repository and i can't add the newline to the PR (it's my first time working with forks and things got a little messy there, so i deleted the old one and did a fresh fork).

LoicPoullain commented 5 years ago

Hi @Jrfidellis ,

Thanks for submitting the PR.

I guess that this one cannot be modified anymore since the "compare" branch no longer exists (the old fork has been deleted). The only solution that I see here is to recreate a new branch from your fresh fork and submit a new PR. If you have other problems while working with forks in the future, don't hesitate to tell me, maybe I can help!

Regarding the missing newline error, it is due to the rules I defined with TSLint. You can configure VSCode to highlight these errors when writing code and even fix them when saving your files. I wrote a small tuto in the docs on how to do that.

LoicPoullain commented 5 years ago

Fixed in v0.6.0