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

test: increase timeouts on arm #1366

Closed
wants to merge 6 commits into from

Conversation

silverwind
Copy link
Contributor

Related: #1343

The static factors are still subject to change, and I think I'll look into benchmarking to obtain them dynamically. ARMv8 gets no special treatment as I think it won't need it. The ARMv6 factor is probably too low for some boards. My RPi does fine with around 2-3x, but it is overclocked and has a pretty fast SD.

If someone could start off a CI run with this PR, I'd be grateful as this should green out CI for ARMv6 and ARMv7.

@@ -51,7 +51,7 @@ function onNoMoreLines() {

setTimeout(function testTimedOut() {
assert(false, 'test timed out.');
}, 6000).unref();
}, common.platformTimeout(3000)).unref();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This timeout seems to have been chosen too generous, so I reduced it.

@silverwind silverwind added the test Issues and PRs related to the tests. label Apr 7, 2015
@silverwind
Copy link
Contributor Author

@bnoordhuis could you start off a CI run with this PR? I need it to determine if the factors need more tweaking.

@bnoordhuis
Copy link
Member

@silverwind
Copy link
Contributor Author

Results are looking quite promising. I've rebased my branch to include silverwind@92ab3c0 which should fix two --- TIMEOUT --- cases and increased the timeout on one last problematic test. This will only leave one failing test on the iojs-armv7-wheezy bot, which is certainly not timeout related:

=== release test-child-process-exit-code ===
Path: parallel/test-child-process-exit-code
assert.js:88
  throw new assert.AssertionError({
        ^
AssertionError: 'SIGILL' === null

@bnoordhuis would appreciate another CI run!

(also I think I messed up this PR with my last rebase)

@bnoordhuis
Copy link
Member

@silverwind You should probably sort out the rebase before I start another CI run, it looks like a wrong merge base now.

@silverwind
Copy link
Contributor Author

@bnoordhuis branch fixed.

@bnoordhuis
Copy link
Member

@silverwind
Copy link
Contributor Author

Quite satisfied so far. Results:

  • iojs-raspbian-wheezy-pi1 passing for the first time
  • iojs-armv7-ubuntu1401 passing (did already pass before)
  • iojs-raspbian-wheezy-pi2 would've passed if it didn't crash in an unrelated test:
=== release test-deprecation-flags ===
Path: sequential/test-deprecation-flags
Command: out/Release/iojs /home/iojs/build/workspace/iojs+any-pr+multi/nodes/iojs-armv7-wheezy/test/sequential/test-deprecation-flags.js
--- CRASHED ---
  • iojs-raspbian-wheezy-pi2 (armv6) also would've passed but ran into a --- TIMEOUT --- (seems 3066f2c isn't enough for that test)
 === release test-stringbytes-external ===
 Path: parallel/test-stringbytes-external
 Command: out/Release/iojs /home/iojs/build/workspace/iojs+any-pr+multi/nodes/iojs-raspbian-wheezy-pi2/test/parallel/test-stringbytes-external.js
 --- TIMEOUT ---

@bnoordhuis please review.

@silverwind silverwind changed the title test - increase timeouts on ARM test: increase timeouts on arm Apr 8, 2015
armv7: 2.0
};

exports.platformTimeout = function (ms) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor style issue: function(ms)

@bnoordhuis
Copy link
Member

Left some comments.

@silverwind
Copy link
Contributor Author

In addition to above comments, I'll work on fixing that last timeout from iojs-raspbian-wheezy-pi2 by adding a armv6 specific factor to tools/test.py.

@silverwind
Copy link
Contributor Author

@bnoordhuis good for another CI run.

iojs-raspbian-wheezy-pi2 should go blue now, and with a bit of luck, all four bots should succeed.

@rvagg
Copy link
Member

rvagg commented Apr 9, 2015

FYI, the persistent failure of parallel/test-debug-port-cluster on the ARMv8 machine was because there was a long running process there from a previous test run that was blocking the debug port that test was trying to use. I've killed that and now:

armv8

So woohoo! I think this means we have solid ARMv8 support now doesn't it? I thought OpenSSL 1.0.2 was a blocker for us on ARMv8 but obviously that was addressed, by @bnoordhuis I guess. Unless anyone has a reason not to, I'm going to push forward trying to get some more server resources to figure out a release build strategy for ARMv8/AARCH64 binaries.

@silverwind
Copy link
Contributor Author

@rvagg 🎉 nice, that means armv8 works pretty well already. While you're at the CI, I think the ARM bots could use some relabeling. You mentioned iojs-raspbian-wheezy-pi2 is armv7, but it's actually armv6. I suggest including the architecture name for all bots, to avoid confusion.

@shigeki
Copy link
Contributor

shigeki commented Apr 9, 2015

😄 Glad to see a blue light on armv8! Wait for a moment for ssl.

@bnoordhuis
Copy link
Member

@shigeki
Copy link
Contributor

shigeki commented Apr 9, 2015

Woops, I'll fix this.

@shigeki
Copy link
Contributor

shigeki commented Apr 9, 2015

Sorry I missed the issue. I've just abort my job. So 472 will go on right now.

@rvagg
Copy link
Member

rvagg commented Apr 9, 2015

@silverwind derp! I have an origial pi1 switched with the pi2 and they are reporting themselves as such, in the process of rectifying now

@silverwind
Copy link
Contributor Author

Almost fully blue 😃

screenshot_43

The iojs-raspbian-wheezy-pi2 failure looks to be another unrelated issue:

=== release test-net-settimeout ===
Path: parallel/test-net-settimeout
events.js:141
      throw er; // Unhandled 'error' event
            ^
Error: read ECONNRESET
    at exports._errnoException (util.js:749:11)
    at TCP.onread (net.js:538:26)
Command: out/Release/iojs /home/iojs/build/workspace/iojs+any-pr+multi/nodes/iojs-raspbian-wheezy-pi2/test/parallel/test-net-settimeout.js

@bnoordhuis please review so I can land this.

@bnoordhuis
Copy link
Member

LGTM

@rvagg
Copy link
Member

rvagg commented Apr 9, 2015

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/473/ should be with the right machines running in the right place

@silverwind
Copy link
Contributor Author

Looks iojs-raspbian-wheezy-pi1 is still in the make stage after 1hr20, which is different than before. I won't let that CI run hold up this PR now, but will check the results later (I expect that armv6 job to take 5-6hrs, because that's what it takes on my RPi1).

silverwind added a commit that referenced this pull request Apr 9, 2015
This commit introduces platform-specific test timeouts for the ARM
architectures. ARMv6 is notoriously slow so gets very large timeouts on
both the timeout value for each test, as well as certain problematic
individual tests. ARMv7 and ARMv8 also get slightly increased headroom.

PR-URL: #1366
Fixes: #1343
Reviewed-By: Ben Noordhuis <[email protected]>
@silverwind
Copy link
Contributor Author

Merged in 7049d7b

@silverwind silverwind closed this Apr 9, 2015
@Fishrock123
Copy link
Contributor

@silverwind iirc, ccache is running on those machines, so it should be less than if you are running without.

@silverwind
Copy link
Contributor Author

Little Pi still going strong after 7hrs:

https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/473/nodes=iojs-raspbian-wheezy-pi1/

(I think it's safe to assume there's no cache involved here)

@rvagg
Copy link
Member

rvagg commented Apr 11, 2015

it's because I changed the env of this and pi2 when you notified me they were around the wrong way - so the cache has to start again with new env vars influencing the build .. over time it should get much better

@rvagg rvagg mentioned this pull request Apr 11, 2015
rvagg added a commit that referenced this pull request Apr 14, 2015
Notable changes:

* C++ API: Fedor Indutny contributed a feature to V8 which has been
  backported to the V8 bundled in io.js. SealHandleScope allows a C++
  add-on author to seal a HandleScope to prevent further, unintended
  allocations within it. Currently only enabled for debug builds of
  io.js. This feature helped detect the leak in #1075 and is now
  activated on the root HandleScope in io.js. (Fedor Indutny) #1395.
* ARM: This release includes significant work to improve the state of
  ARM support for builds and tests. The io.js CI cluster's ARMv6,
  ARMv7 and ARMv8 build servers are now all (mostly) reporting passing
  builds and tests.
  - ARMv8 64-bit (AARCH64) is now properly supported, including a
    backported fix in libuv that was mistakenly detecting the
    existence of `epoll_wait()`. (Ben Noordhuis) #1365.
  - ARMv6: #1376 reported a problem with Math.exp() on ARMv6 (incl
    Raspberry Pi). The culprit is erroneous codegen for ARMv6 when
    using the "fast math" feature of V8. --nofast_math has been turned
    on for all ARMv6 variants by default to avoid this, fast math can
    be turned back on with --fast_math. (Ben Noordhuis) #1398.
  - Tests: timeouts have been tuned specifically for slower platforms,
    detected as ARMv6 and ARMv7. (Roman Reiss) #1366.
* npm: Upgrade npm to 2.7.6. See the release notes
  (https://github.com/npm/npm/releases/tag/v2.7.6) for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants