http-server
http-server copied to clipboard
Upgrade tap
Update tap and fix deprecated test methods
What changes did you make? Upgraded to latest tap and replaced deprecated test methods in the unit tests.
Provide some example code that this change will affect, if applicable:
request.get(`http://localhost:${port}/a.txt`, (err, res) => {
- t.ifError(err);
+ t.error(err);
t.equal(res.statusCode, 200, 'a.txt should be found');
t.equal(res.headers['cache-control'], 'max-whatever=3600');
requestAsync("http://localhost:8080/").then(res => {
t.ok(res);
t.equal(res.statusCode, 200);
- t.includes(res.body, './file');
- t.includes(res.body, './canYouSeeMe');
+ t.match(res.body, './file');
+ t.match(res.body, './canYouSeeMe');
Is there anything you'd like reviewers to focus on? Hopefully the changes are pretty straightforward, but note that whilst test coverage remains the same the newer version of tap reports anything less than 100% coverage as an error by default:
ERROR: Coverage for lines (87.02%) does not meet global threshold (100%)
ERROR: Coverage for functions (89.29%) does not meet global threshold (100%)
ERROR: Coverage for branches (77.53%) does not meet global threshold (100%)
ERROR: Coverage for statements (86.66%) does not meet global threshold (100%)
To avoid this I have added the no-check-coverage
flag to the npm scripts inside package.json.
Please ensure that your pull request fulfills these requirements:
- [X] The pull request is being made against the
master
branch - [X] Tests for the changes have been added (for bug fixes / features)
- [ ] New features/options have been documented in:
- [ ]
http-server --help
text - [ ] README.md
- [ ] doc/http-server.1
- [ ]
What is the purpose of this pull request? (bug fix, enhancement, new feature,...) enhancement and security fixes (dependent packages)
Fixed package-lock
Haha i think your package-lock fix from #744 is conflicting here again
Haha i think your package-lock fix from #744 is conflicting here again
It's the never-ending struggle, should be fixed now
Not sure why some tests are passing whilst others are failing based on OS and node js versions...
Yeah we've been having some issues with non-deterministic errors, I made them a little better recently but they still pop up. Check out #723 if you have any insight.
For now I'm just re-running them to see if that works
For some reason this PR is just not letting tests pass, the same non-deterministic errors are happening every time, where normally it's only ever 10 runs or so on master or other PRs, so maybe something in this new tap version is triggering them harder.
I (or anyone who jumps in first) am going to take a look at the tests over the next few days and see what I can do, and I'm going to let this PR just sit until then, because I don't want to make the tests harder in master. If you're doing this for Hacktoberfest I can do a one-off acceptance so you still get credit, just let me know, otherwise this may sit for a bit.
@thornjad - I don't need any more Hacktoberfest approvals, so don't worry about that; I agree that you shouldn't merge this until we at least figure out why the tests are failing.
In the meanwhile I'll see if I can reproduce the issues on macOS with older NodeJS as that seems to be the most common culprit. Might be relating to clean-up of resources if it's a port in use, perhaps Tap updates are no longer cleaning up like they used to or somehow exacerbating an existing issue.
Apologies, I lost track of this and feel it's probably or of date enough to forget about now.