| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Please could you format your commit message according to the Contributing Guidelines? Maybe something like this? test: remove extra arg from test-module-loading |
Sorry, something went wrong.
|
One question I had was regarding this. I wanted to add a second argument (a reg exp) that matched the error message, but I'm not sure what the error message is. |
Sorry, something went wrong.
|
Just modified the commit message also. |
Sorry, something went wrong.
|
@tarang9211 to find out the error message, simply remove the assert.throws() wrapper and run the test and see what error is reported :-) |
Sorry, something went wrong.
|
@jasnell so basically just this function() { require('./utils') } ? |
Sorry, something went wrong.
|
... just requires('./utils') |
Sorry, something went wrong.
|
hey @jasnell thanks. here's a pastebin of the logs. Is it the string test name clashes or Cannot find module './utils' ? If I add that as a second argument, the tests still fail. |
Sorry, something went wrong.
|
You'd want, assert.throws(function() { require('./utils'); },
/^Error: Cannot find module '\.\/utils'$/);
|
Sorry, something went wrong.
|
Yup I tried that, too. I get the same output log as the pastebin above. assert.throws(function() { require('./utils'); },
/^Error: Cannot find module '.\/utils'$/);The regex is appropriate as I got it to validate. |
Sorry, something went wrong.
|
@tarang9211 The output log is normal, as long as you don't have a stack trace at the end. I tried running the test with your regexp and it works so I think you can push the change. |
Sorry, something went wrong.
|
@targos sounds good, thanks for the update. Do have a look at the most recent commit in that case. |
Sorry, something went wrong.
Sorry, something went wrong.
|
@jasnell Do I break something :O ? |
Sorry, something went wrong.
|
@tarang9211 looks like an infrastructure flake, let's try again: |
Sorry, something went wrong.
|
@gibfahn one more test left 😄 |
Sorry, something went wrong.
|
@tarang9211 If you click on the link you'll see everything passed. The test/arm failure in the GitHub UI is a reporting infra issue. See nodejs/build#572 |
Sorry, something went wrong.
|
@gibfahn Oh alright, thanks clears it up. So this is ready to be merged? |
Sorry, something went wrong.
Also removes extraneous argument. PR-URL: #11413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
|
Landed in a4c3e31 I squashed the commits and modified the commit message, @tarang9211 in future you could try doing this yourself (not required but helpful!) Thanks and congrats on your first PR to node! |
Sorry, something went wrong.
|
@gibfahn sweet! So has this been merged into master, yet? |
Sorry, something went wrong.
|
@tarang9211 Yes, if you look at the commit I posted above (a4c3e31) you'll see that it's in master. We don't merge commits in because that creates an extra merge commit (which isn't necessary). So if you see the Landed in <commit sha> comment, you know the PR has landed. |
Sorry, something went wrong.
Also removes extraneous argument. PR-URL: #11413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Also removes extraneous argument. PR-URL: #11413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Also removes extraneous argument. PR-URL: #11413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Also removes extraneous argument. PR-URL: #11413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Also removes extraneous argument. PR-URL: #11413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
| Back | FazBrowse Home | New Git URL |
First contribution to the node repo. This PR provides a small fix to the test-module-loading test case.
Checklist