Tests fail on cloned repository, even after running "npm install"

[TehShrike]

contrib.src.js doesn't exist in any directory, and the following line trying to load asq.js fails as well (unless you run build first, which I didn't figure should be necessary).

Tested on OSX on node 0.10.32:

...
Core Test #23: PASSED
Core Test #24: PASSED
ALL CORE TESTS PASSED!


module.js:340
    throw err;
          ^
Error: Cannot find module '/Users/josh/code/asynquence/contrib/contrib.src.js'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/josh/code/asynquence/contrib/node-tests.js:14:11)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
*** TEST SUITE FAILURE ***

[getify]

This would only happen if you got the package from github only, and not from npm. Since most people use npm to get things, and especially since most people use npm/node for the testing, it seems reasonable to expect that people would either get the code from npm, OR they would run npm install on the downloaded git package to install its dependencies (which btw would also have retrieved the built asq.js).

I would agree that the documentation needs to be clearer on this detail. I should add something like how this section treats it:

https://github.com/getify/native-promise-only#testscompliance

[TehShrike]

Sorry, I should have specified my install process.

I cloned the repository with git, and then ran npm install in the asynquence directory before running the npm test that produced the above output.

npm install in a directory with a package.json just downloads the dependencies, it doesn't overwrite the directory with the package contents from npm.

[legodude17]

The problem is that npm install. Will not build the contrib, and the built contrib is required for the tests. My PR will make this easier: #91

[getify]

Again, you shouldn't be running "npm *" commands unless you retrieved the code from npm. There's bound to be mismatches when doing so.

[TehShrike]

What? I must be misunderstanding you. git clone repository; npm install seems to be standard setup/test process in the node/JavaScript community. Are you saying that you don't expect anybody to be doing that?

What is the setup/test process that you expect contributors to follow?

[getify]

Maybe that is a "standard". But it's never been something I did, not even once. So no, I didn't design or structure the repo and packages that way.

If you want to git clone + npm *, then you need to remember that this git repo actually includes two separate npm package roots in it. So, roughly, I think you would need to do npm install in the root of this repo, and also npm install in the contrib/ dir. IIUC, that should build both the core and the contrib.

I know that npm install doesn't have the precedent to recursively run in subdirectories, b/c it's probably not common to have a git repo with two npm package roots in it.

Keep in mind that if you get asynquence from npm, you only get the core... there's no subdir for the contrib package. So through npm usage, you end up with asynquence and asynquence-contrib as side-by-side node_modules directories, not the latter inside the former.

I don't want to make the use-case of getting-and-using-from-npm awkward or difficult. So the package.json setup is designed to support that, primarily. If you go the git route, that's fine, but it works differently, and requires a little more work.

[TehShrike]

So if someone wanted to be able to run the tests from git and be able to contribute pull requests to the Github repo, what process should they follow?

[getify]

I think I would advise following this:

1. git clone ..
2. in /, npm install
3. in contrib/, npm install
4. in /, npm test
5. ...

Is there something about that I'm missing which makes it not work? I was under the impression that the complaint is that the first npm install doesn't automatically do the second one, which I've tried to explain why.

[TehShrike]

Yeah, the issue is that I never knew that I had to do the step where you navigate to contrib and run npm install - it wasn't in the readme, and I wasn't able to derive that extra step from the scripts. I think if you add a "How to contribute" section to the readme then you'll be covered.

As long as "how to contribute/develop" is in the readme, I don't care how many npm installs are in it.

[getify]

Nods. Yeah that was my original plan i just haven't gotten around to it. Good reminder tho. :)

[legodude17]

Something that would be very helpful is some sort of task file with one of
many different task managers.

On Thu, Jul 28, 2016 at 12:50 PM, Kyle Simpson notifications@github.com
wrote:

Nods. Yeah that was my original plan i just haven't gotten around to it.
Good reminder tho. :)

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#54 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKr56PZDR2Cf8ayRsryv97gEXStJ27tpks5qaQfugaJpZM4CtHHA
.

[legodude17]

There should be a contribution section: #93. I can do that once we agree on the best way to lay it out.

[legodude17]

Another option would be like a build all type file, like build-all.js that would run npm install in both. And then it could be added to .npmignore.

[getify]

Yeah, that might work. Wouldn't be an npm script, so people would have to run it manually.

[legodude17]

I will look into it.

On Fri, Jul 29, 2016 at 12:20 PM, Kyle Simpson notifications@github.com
wrote:

Yeah, that might work. Wouldn't be an npm script, so people would have to
run it manually.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#54 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKr56PeVTeSnRivBi6XHM-5qJUl9mDCCks5qalJpgaJpZM4CtHHA
.

[legodude17]

Finished: #94.