-
-
Notifications
You must be signed in to change notification settings - Fork 597
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
Fix loader-generator and plugin-generator tests #1250
Fix loader-generator and plugin-generator tests #1250
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We got rid of lock files since we use yarn workspaces now, please use yarn and remove the lock files.
49b7a32
to
708dabe
Compare
fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it's not fixed yet.
Look at the logs in the CI. I believe the assertion library doesn't work well with jest OR the assertions are not correct.
npm ERR! code ENOENT
npm ERR! syscall open
npm ERR! path /tmp/c12aae677bfdb92438557a5218666476c5e2f591/my-loader/package.json
npm ERR! errno -2
npm ERR! enoent ENOENT: no such file or directory, open '/tmp/c12aae677bfdb92438557a5218666476c5e2f591/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-22T08_18_01_051Z-debug.log
1cfb720
@Loonride Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ematipico Please review the new changes. |
1cfb720
to
a0504c0
Compare
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
It turns out this had nothing to do with the test itself, the generators needed to be writing a package.json but were not. |
Exactly, and the weird bit it that the tests are passing, it should fail in case it fails to write the files. |
So we've found a bug! Nice!! 🙏 |
@@ -21,7 +21,7 @@ const PluginGenerator = addonGenerator( | |||
validate: (str: string): boolean => str.length > 0 | |||
} | |||
], | |||
path.resolve(__dirname, "..", "generate-plugin"), | |||
path.resolve(__dirname, "../../generate-plugin/templates"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this! That was my fault
What kind of change does this PR introduce?
test fix
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
N/A
Summary
There were some problems with the
loader-generator
andplugin-generator
test files which I fixed.Does this PR introduce a breaking change?
No
Other information