search

chaijs / chai-http

HTTP Response assertions for the Chai Assertion Library.
http://chaijs.com/plugins/chai-http
632 stars 112 forks source link

Where is the lib-cov/http #117

Closed gustavoisensee closed 7 years ago

gustavoisensee commented 7 years ago

Hi,

I was doing automatization tests with karma, phantom on gitlab and my tests didn't pass. So I've received the message saying:

Module not found: Error: Cannot resolve 'file' or 'directory' ./lib-cov/http in D:\DEV\GIT\Interno\HoursBankAPI\node_modules\chai-http @ ./~/chai-http/index.js 1:72-97

so, firstly I'm sorry, I'm new with chai-http but I I haven't found anything in the documentation about lib-cov, could you explain me what is it and how is it works?

meeber commented 7 years ago

@gustavoisensee Sorry for the delay. It looks like to me that these lines are the relics of some old build process that's no longer in place, and should be changed to module.exports = require('./lib/http');.

Thoughts @keithamus?

keithamus commented 7 years ago

Yes I think we could do with a bit of an overhaul of the build process. We're using istanbul now which doesn't require this, AFAIK.

Let's change this to PR-wanted, and easy fix:

The change:

We need drop the require('./lib-cov/http') call in index.js as we're no longer using it as part of our build setup.

Who can make this PR?

@oluoluoxenfree has first refusal on this issue, as they elected to solve https://github.com/chaijs/chai-http/issues/75 but it was resolved before they could. If @oluoluoxenfree choses not to work on this issue, then we'll open it out to the broader community, welcoming new and yet-to-be contributors.

I want to make this PR!

Great! If @oluoluoxenfree doesn't want to work on this one, and you are a new or yet-to-be contributor then you are welcome to! You should take a look at our Code of Conduct and Contribution Guidelines (note: we need to work on our contribution guidelines, so these might not apply exactly to this sub-project).

If you want to work on this, you should comment below, something like:

Hey, I'd like to work on this issue, I'll be making a PR shortly.

If you don't comment you might run the risk of someone else doing the work before you!

If someone has already commented, then you should leave this PR for them to work on.

If someone has commented, but a few weeks have passed - it is worth asking the person who has commented if they're still interested in working on this, and if you can pick it up instead.

How to make a PR for this change:

This one should just be a case of removing the ternary operator and additional require call from index.js. The new contents of index.js should look like:

module.exports = require('./lib/http');

Once if you done this, if you run the npm test command then you should see it all passes and a coverage report is still generated. If the coverage report shows that there is no coverage, then something went wrong. If you get stuck, just pop on here to explain the problem and the @chaijs/chai-http will try to help with the issue.

What happens then?

We'll need 2 contributors to accept the change, and it'll be merged. Once merged, we'll need to take some time to make a manual release, which will take a while - as we need to prepare release notes. It'll likely be released as part of chai-http@4.0.0 because we have outstanding work from https://github.com/chaijs/chai-http/issues/75 that needs to be merged.

bartw commented 7 years ago

Hello,

I would like to do this minor fix. I'll have pr ready soon.

vieiralucas commented 7 years ago

Fixed by #136

alexey-pelykh commented 7 years ago

When this will go to release?