tools: fix Galaxy discovery #8709
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1a03655 in 1.4.4.2 expanded on the HTTP error checking added by 30aec9f in 1.4.2. Neither of these changes were aware that discoverGalaxy invokes httpHelpers.request with json:true, resulting in a `body` that is a parsed JSON object rather than a string or Buffer. Before 1.4.4.2, this had no consequences because body.length is undefined and `undefined < 90` is false, but the change to Buffer.byteLength actually made the condition true. It's safe to not check length in the JSON case because a truncated JSON object is not legal JSON (unless the truncation just drops trailing whitespace, in white case that's OK). I check for both string and Buffer because some calls to this function pass in an encoding option. Buffer.byteLength works with both types.
These checks should still happen when body is an empty string, which (for better or worse) is falsy in JavaScript.
benjamn
approved these changes
May 21, 2017
added 3 commits
May 21, 2017 11:43
To deal with individual flaky tests, we often just re-run the entire test suite, which feels like an enormous waste of shared computing resources. This change automatically re-runs individual failed tests as many as two more times, and considers the test successful if any of those attempts succeeds. cc @abernix @hwillson et al.
This method appears to be causing large spikes in memory consumption on Circle CI during the `meteor --get-ready` preparation step, which often leads to the test process being killed. Also added a call in IsopackCache#_loadLocalPackage for good measure. We're now calling requestGarbageCollection pretty frequently when we run Node with --expose-gc, but that currently only happens during Circle CI tests, so I don't think we need to implement the improvements suggested in tools/utils/gc.js, yet. Previously: 35f488e, f6df21f
In the ongoing struggle with Circle CI-specific test failures, the preparatory `meteor --get-ready` has been a consistent point of failure, before any real tests have the chance to run. Using a lighter-weight command (meteor --help) that still does most of what --get-ready did seems worth a try, though it might just defer memory-intensive work until later, so we'll have to see what happens.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1a03655 in 1.4.4.2 expanded on the HTTP error checking added by 30aec9f in
1.4.2. Neither of these changes were aware that discoverGalaxy invokes
httpHelpers.request with json:true, resulting in a
bodythat is a parsed JSONobject rather than a string or Buffer. Before 1.4.4.2, this had no consequences
because body.length is undefined and
undefined < 90is false, but the changeto Buffer.byteLength actually made the condition true.
It's safe to not check length in the JSON case because a truncated JSON object
is not legal JSON (unless the truncation just drops trailing whitespace, in
white case that's OK).
I check for both string and Buffer because some calls to this function pass in
an encoding option. Buffer.byteLength works with both types.