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

Encoding problems when running the cli tests on Windows #207

Closed
LoicPoullain opened this issue Sep 7, 2018 · 12 comments
Closed

Encoding problems when running the cli tests on Windows #207

LoicPoullain opened this issue Sep 7, 2018 · 12 comments

Comments

@LoicPoullain
Copy link
Member

LoicPoullain commented Sep 7, 2018

Issue

Issue reported in the PR #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

  • Ask collaborators to use a virtual machine (for example vagrant) when developing for the project.

OR

  • Update test-environment.ts to replace all \r\n characters by \n when loading a generated file during tests (maybe easier).
@LoicPoullain LoicPoullain changed the title Fix encoding problems when running the cli tests on Windows Encoding problems when running the cli tests on Windows Sep 7, 2018
@Jrfidellis
Copy link
Contributor

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
Copy link
Member Author

LoicPoullain commented Sep 13, 2018

@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
Copy link
Member Author

LoicPoullain commented Sep 13, 2018

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

image

@LoicPoullain
Copy link
Member Author

LoicPoullain commented Oct 2, 2018

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
Copy link
Contributor

Jrfidellis commented Oct 10, 2018

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
Copy link
Member Author

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:

  • In package.json, NODE_ENV=test mocha or NODE_ENV=test&& mocha become mocha --file "./src/test.ts"

  • // ./src/test.ts
    process.env.NODE_ENV = 'test';

@Jrfidellis
Copy link
Contributor

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
Copy link
Member Author

LoicPoullain commented Oct 22, 2018

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
Copy link
Member Author

Fixed in v0.6.0

Issue tracking automation moved this from In Progress to Done Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Issue tracking
  
Done / Closed This Release
Development

No branches or pull requests

2 participants