Skip to content
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

Improve test-timeout reliability #2414

Merged
merged 1 commit into from Oct 27, 2016

Conversation

Projects
None yet
2 participants
@mscdex
Copy link
Contributor

commented Oct 14, 2016

This commit should allow the 'connect timeout' test to pass more often than not.

@mscdex mscdex force-pushed the mscdex:tests-timeout-reliability branch from 6417be8 to 4252d86 Oct 20, 2016

@mscdex mscdex force-pushed the mscdex:tests-timeout-reliability branch from 4252d86 to 5c93d8c Oct 20, 2016

@mscdex

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2016

@simov Does this LGTY? I'm still encountering this locally occasionally. I have also seen it on Travis before FWIW.

@simov simov merged commit 76d909f into request:master Oct 27, 2016

3 checks passed

codecov/patch Coverage not affected when comparing 8c04a23...5c93d8c
Details
codecov/project 92.32% (+0.00%) compared to 8c04a23
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mscdex mscdex deleted the mscdex:tests-timeout-reliability branch Nov 15, 2016

pulkitsinghal added a commit to ShoppinPal/vend-nodejs-sdk that referenced this pull request Sep 15, 2017

Update dependencies `request` and `request-promise` ... all tests passed
## Motivation
- Some upstream projects that use `vend-nodejs-sdk` were facing strange
issues where long-running use (after 47 minutes or so) led so a situation
where nodejs would just hang after a POST request.
- There weren't any clear CPU or Memory consumption red flags.
- It seemed more like a bad promise chain or failing to invoke a callback
that often leads nodejs into a forever running state and there's nothing
really going on.

## Solution
Look at the [CHANGELOG](https://github.com/request/request/blob/master/CHANGELOG.md)
for `request` and see if any plausible reasons for trying a module update.

1. request/request#2431
2. request/request#2414
3. request/request#2447
4. request/request#2448

This wasn't an exact science but their descriptions seemed to merit
the dependency upgrade as "at least worth a try"

## Consequences
Dropped support for Node 0.10
request/request#2381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.