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

tests(generator): enhance init generator tests #1236

Merged
merged 6 commits into from
Feb 23, 2020

Conversation

anshumanv
Copy link
Member

enhance init generator tests

What kind of change does this PR introduce?
tests

Did you add tests for your changes?
Yay

If relevant, did you update the documentation?
NA

Summary

  • Added file and content checks in the generated files in init generator, missed that in my old PR.

Does this PR introduce a breaking change?
No

Other information

@webpack-bot
Copy link

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

ematipico
ematipico previously approved these changes Feb 17, 2020
jamesgeorge007
jamesgeorge007 previously approved these changes Feb 17, 2020
rishabh3112
rishabh3112 previously approved these changes Feb 17, 2020
Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Your test is failing. Check the logs.

@@ -1,7 +1,7 @@
import { join } from 'path';
import { run } from 'yeoman-test';
const assert = require('yeoman-assert');
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the import. You are under TypeScript here

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried it but it wasn't working with this package. :(

Copy link
Contributor

Choose a reason for hiding this comment

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

That's probably because you didn't install the types

@@ -1,7 +1,7 @@
import { join } from 'path';
Copy link
Contributor

Choose a reason for hiding this comment

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

You should install also @types/yeoman-assert

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, will make the changes, good catch. 👍


// Check that all the project files are generated with the correct name
const filePaths = ['package.json', 'README.md', 'src/index2.js'];
assert.file([...filePaths.map(file => join(outputDir, file))]);
Copy link
Contributor

@ematipico ematipico Feb 17, 2020

Choose a reason for hiding this comment

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

I had to skip the other tests that I merged because they were failing. You should use the jest assertion in order to tell to jest that is working. Or checking yeoman-assert documentation to understand how to integrate it with jest

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, sounds fair, will take a look. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it a problem with yeoman-assert? I can't seem to find a way to integrate them with jest and the docs are really minimal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Triggered travis on next again and it seems to pass now. 😟🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Check the logs locally while you test them (maybe run that single test). There are errors and jest can't recognize any assertion so it's like the test is empty

Copy link
Contributor

Choose a reason for hiding this comment

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

At this point do not use yeoman-assert and stick with the normal expect

Copy link
Member Author

Choose a reason for hiding this comment

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

Check the logs locally while you test them (maybe run that single test). There are errors and jest can't recognize any assertion so it's like the test is empty

I just ran a test which was supposed to fail in yeoman-assert and jest caught it, I think the errors from yeoman-assert are caught by jest on exit.

@ematipico
Copy link
Contributor

This is from the logs

PASS packages/generators/__tests__/loader-generator.test.ts (5.522s)
npm ERR! code ENOENT
npm ERR! syscall open
npm ERR! path /tmp/f9ff699a773feb35ce68ffe10cd55974e6943e1c/my-webpack-plugin/package.json
npm ERR! errno -2
npm ERR! enoent ENOENT: no such file or directory, open '/tmp/f9ff699a773feb35ce68ffe10cd55974e6943e1c/my-webpack-plugin/package.json'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent 

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/2020-02-17T09_25_15_716Z-debug.log

ℹ INFO  For more information and a detailed description of each question, have a look at: https://github.com/webpack/webpack-cli/blob/master/INIT.md
ℹ INFO  Alternatively, run "webpack(-cli) --help" for usage info

npm ERR! code ENOENT
npm ERR! syscall open
npm ERR! path /tmp/166db8056f244410d75c82f0f8f7e83bcb726108/my-loader/package.json
npm ERR! errno -2
npm ERR! enoent ENOENT: no such file or directory, open '/tmp/166db8056f244410d75c82f0f8f7e83bcb726108/my-loader/package.json'
npm ERR! enoent This is related to npm not being able to find a file.
npm ERR! enoent 

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/2020-02-17T09_25_16_276Z-debug.log

@anshumanv
Copy link
Member Author

This is from the logs

Thanks! I wonder how it's failing now when it was fine before. Unusual. Anyways, will take a look. 👍

@anshumanv anshumanv closed this Feb 17, 2020
@anshumanv anshumanv reopened this Feb 17, 2020
@anshumanv
Copy link
Member Author

Why is snyk failing? Does anybody have any ideas around it? And what packages is it failing for?

@ematipico
Copy link
Contributor

I merged the PR where we moved the main cli under lerna. Also, we switched to yarn. You should use that now :)

@anshumanv
Copy link
Member Author

I merged the PR where we moved the main cli under lerna. Also, we switched to yarn. You should use that now :)

Thanks for the heads up! Will rebase and do the changes in a while. Thanks!

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Is the assertion now working?

@rishabh3112 rishabh3112 changed the title tests(generator): enahnce init generator tests tests(generator): enhance init generator tests Feb 23, 2020
@webpack-bot
Copy link

@rishabh3112 Thanks for your update.

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

@ematipico Please review the new changes.

@rishabh3112
Copy link
Member

Thanks!

@anshumanv anshumanv deleted the test/init-gen branch February 23, 2020 07:38
@anshumanv
Copy link
Member Author

Thanks for the rebase! 😄

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.

6 participants