build: run cctests as part of test-ci target #8034

bnoordhuis · 2016-08-09T15:25:45Z

For discussion.

CI: https://ci.nodejs.org/job/node-test-pull-request/3591/

bnoordhuis · 2016-08-09T15:56:56Z

Different take that moves merging of the TAP files out of tools/test.py and into a standalone script: https://ci.nodejs.org/job/node-test-pull-request/3592/

richardlau · 2016-08-09T16:13:16Z

tools/merge-tap.py

+      head, rest = rest.split('\n', 1)
+    if head.startswith('1..'):
+      total += int(head[3:])
+      output += rest


Will this work without renumbering the individual tests in the files being merged?
As it stands I think we'll end up with multiple ok 1 ....

jbergstroem · 2016-08-09T16:50:05Z

cc @nodejs/build

joaocgreis · 2016-08-10T04:32:43Z

On Windows, vcbuild test-ci already runs cctest but ignores the results. Here is the error from a Debug build:

Running main() from gtest_main.cc
[==========] Running 22 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 4 tests from UtilTest
[ RUN      ] UtilTest.ListHead
[       OK ] UtilTest.ListHead (0 ms)
[ RUN      ] UtilTest.StringEqualNoCase
[       OK ] UtilTest.StringEqualNoCase (0 ms)
[ RUN      ] UtilTest.StringEqualNoCaseN
[       OK ] UtilTest.StringEqualNoCaseN (0 ms)
[ RUN      ] UtilTest.ToLower
[       OK ] UtilTest.ToLower (0 ms)
[----------] 4 tests from UtilTest (0 ms total)

[----------] 18 tests from InspectorSocketTest
[ RUN      ] InspectorSocketTest.ReadsAndWritesInspectorMessage
Assertion failed: backlog > 0, file src\win\tcp.c, line 561

bnoordhuis · 2016-08-10T10:00:50Z

Removed the script again, made changes to vcbuild.bat and added a -Wformat fix for the inspector cctest because I didn't feel like filing a separate PR for that. :-)

New CI: https://ci.nodejs.org/job/node-test-pull-request/3609/

@ofrobots @eugeneo Running the cctests seems to have unearthed some issues. Several of the buildbots fail like this (from https://ci.nodejs.org/job/node-test-commit-linux/4529/nodes=debian8-x86/console):

# Value of: 0
# Expected: uv_is_active(reinterpret_cast<uv_handle_t*>(&socket))
# Which is: 1
not ok 9 - InspectorSocketTest.ExtraLettersBeforeRequest
  ---
  duration_ms: 0.001
  ...
# Value of: 0
# Expected: uv_is_active(reinterpret_cast<uv_handle_t*>(&socket))
# Which is: 1
not ok 10 - InspectorSocketTest.RequestWithoutKey
  ---
  duration_ms: 0
  ...
# Value of: 0
# Expected: uv_is_active(reinterpret_cast<uv_handle_t*>(&socket))
# Which is: 1
ok 11 - InspectorSocketTest.KillsConnectionOnProtocolViolation

bnoordhuis · 2016-08-23T19:09:25Z

Rebased, let's see if the test failures have been resolved: https://ci.nodejs.org/job/node-test-pull-request/3809/

eugeneo · 2016-08-23T19:34:22Z

I'm working on it - failures are still there...

On Tue, Aug 23, 2016 at 12:10 PM Ben Noordhuis notifications@github.com
wrote:

Rebased, let's see if the test failures have been resolved:
https://ci.nodejs.org/job/node-test-pull-request/3809/

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8034 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AARkrScGJo88I6TjfrW1w9jXvQZkVG6Fks5qi0WggaJpZM4JgM5v
.

eugeneo · 2016-08-23T21:08:36Z

FYI - the test on Windows hit #8155, I'll create a PR once I fix it.

bnoordhuis · 2016-09-11T10:51:47Z

Rebased. New CI run: https://ci.nodejs.org/job/node-test-pull-request/4007/

bnoordhuis · 2016-09-11T11:22:46Z

@eugeneo cctest failures still appear to be unfixed:

not ok 8 - InspectorSocketTest.ExtraTextBeforeRequest
  ---
  duration_ms: 0
  ...
# Value of: 0
# Expected: uv_is_active(reinterpret_cast<uv_handle_t*>(&socket))
# Which is: 1
not ok 9 - InspectorSocketTest.ExtraLettersBeforeRequest
  ---
  duration_ms: 0
  ...
# Value of: 0
# Expected: uv_is_active(reinterpret_cast<uv_handle_t*>(&socket))
# Which is: 1
not ok 10 - InspectorSocketTest.RequestWithoutKey
  ---
  duration_ms: 0
  ...
# Value of: 0
# Expected: uv_is_active(reinterpret_cast<uv_handle_t*>(&socket))
# Which is: 1

eugeneo · 2016-09-14T03:26:21Z

cctest works for me on Windows, Mac and Linux after I apply #8528

bnoordhuis · 2016-09-14T10:27:07Z

CI with fixes from #8505 and #8528 incorporated: https://ci.nodejs.org/job/node-test-pull-request/4044/

EDIT: Green, except for an infrastructure failure on one of the ARM buildbots.

Should help with #8034. PR-URL: #8528 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>

bnoordhuis · 2016-09-23T18:26:03Z

Rebased. New CI: https://ci.nodejs.org/job/node-test-pull-request/4243/

bnoordhuis · 2016-09-23T19:40:11Z

Quite a few flakes but none related to this PR. Can I get a LGTM?

bnoordhuis · 2016-10-03T17:51:40Z

Back-porters, commit b3d283a (the last one) should be omitted when back-porting to v4.x (but not v6.x.)

mscdex · 2016-10-06T05:22:04Z

@bnoordhuis Why did the default result printer switch from pretty print to TAP? I get TAP output from cctest when running make test locally now.

Teach gtest to produce TAP so we can integrate it better with our CI tooling. TAP is printed to stdout but it can also be written to file by passing the `--gtest_output=tap:filename.tap` switch to cctest. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Print size_t and ssize_t using %zd and %zu respectively, not %ld. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Teach gtest to produce TAP so we can integrate it better with our CI tooling. TAP is printed to stdout but it can also be written to file by passing the `--gtest_output=tap:filename.tap` switch to cctest. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Print size_t and ssize_t using %zd and %zu respectively, not %ld. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins · 2016-10-24T16:44:12Z

waiting to backport this until we have consensus on the tap output... has this been decided on master yet?

mscdex · 2016-10-24T20:59:35Z

@thealphanerd The default reporter format change was reverted in master.

MylesBorins · 2016-10-24T21:03:43Z

@mscdex can you give me the PR that happened in?

mscdex · 2016-10-24T21:07:23Z

@thealphanerd #8948

MylesBorins · 2016-11-14T23:08:57Z

@bnoordhuis or @mscdex would one of you be willing to backport this with the commit from #8948?

jbergstroem · 2016-11-15T00:10:56Z

and on top, #9262.

Teach gtest to produce TAP so we can integrate it better with our CI tooling. TAP is printed to stdout but it can also be written to file by passing the `--gtest_output=tap:filename.tap` switch to cctest. PR-URL: nodejs#8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: nodejs#8034 Reviewed-By: James M Snell <jasnell@gmail.com>

mscdex · 2016-11-18T20:44:16Z

@thealphanerd #9682

Teach gtest to produce TAP so we can integrate it better with our CI tooling. TAP is printed to stdout but it can also be written to file by passing the `--gtest_output=tap:filename.tap` switch to cctest. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

bnoordhuis added c++ Issues and PRs that require attention from people who are familiar with C++. build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Aug 9, 2016

nodejs-github-bot added the gtest label Aug 9, 2016

bnoordhuis force-pushed the gtest-tap branch from b5f3b54 to 316e033 Compare August 9, 2016 15:55

richardlau reviewed Aug 9, 2016
View reviewed changes

bnoordhuis force-pushed the gtest-tap branch from 316e033 to db27094 Compare August 10, 2016 09:53

bnoordhuis force-pushed the gtest-tap branch from 0180ce1 to 9720f0f Compare August 23, 2016 19:08

bnoordhuis force-pushed the gtest-tap branch from 9720f0f to a9aa0e5 Compare September 11, 2016 10:51

bnoordhuis mentioned this pull request Sep 12, 2016

Test failures on macOS Sierra 10.12 (GM) with Xcode 8.0 (GM) #8498

Closed

eugeneo mentioned this pull request Sep 14, 2016

inspector: fix tests on Windows #8528

Closed

3 tasks

bnoordhuis force-pushed the gtest-tap branch from a9aa0e5 to b51b463 Compare September 14, 2016 10:24

ofrobots pushed a commit that referenced this pull request Sep 16, 2016

inspector: fix tests on Windows

6f9157f

Should help with #8034. PR-URL: #8528 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>

Trott force-pushed the master branch from b0df363 to c5ce7f4 Compare September 21, 2016 00:09

bnoordhuis force-pushed the gtest-tap branch from b51b463 to bdf3483 Compare September 23, 2016 18:25

bnoordhuis deleted the gtest-tap branch October 3, 2016 17:49

bnoordhuis added the lts-watch-v4.x label Oct 3, 2016

bnoordhuis merged commit b3d283a into nodejs:master Oct 3, 2016

jasnell pushed a commit that referenced this pull request Oct 6, 2016

build: run cctests as part of test-ci target

4cd3090

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request Oct 6, 2016

test: fix -Wformat warnings in inspector cctest

ae2c344

Print size_t and ssize_t using %zd and %zu respectively, not %ld. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016

build: run cctests as part of test-ci target

f057d19

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016

test: fix -Wformat warnings in inspector cctest

424c126

Print size_t and ssize_t using %zd and %zu respectively, not %ld. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added this to the v4.6.2 milestone Oct 24, 2016

MylesBorins modified the milestones: v4.7.0, v4.6.2 Oct 26, 2016

mscdex pushed a commit to mscdex/io.js that referenced this pull request Nov 18, 2016

build: run cctests as part of test-ci target

938e67a

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: nodejs#8034 Reviewed-By: James M Snell <jasnell@gmail.com>

mscdex mentioned this pull request Nov 18, 2016

backport: cctest TAP output changes #9682

Closed

2 tasks

MylesBorins added dont-land-on-v4.x and removed lts-watch-v4.x labels Nov 18, 2016

MylesBorins pushed a commit that referenced this pull request Nov 22, 2016

build: run cctests as part of test-ci target

7c73105

Enable the cctests on the CI now that they know how to write TAP output. PR-URL: #8034 Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins removed this from the 4.7.0 milestone Nov 22, 2016

MylesBorins mentioned this pull request Nov 22, 2016

v4.7.0 proposal #9736

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: run cctests as part of test-ci target #8034

build: run cctests as part of test-ci target #8034

bnoordhuis commented Aug 9, 2016 •

edited

Loading

bnoordhuis commented Aug 9, 2016

richardlau Aug 9, 2016

jbergstroem commented Aug 9, 2016

joaocgreis commented Aug 10, 2016

bnoordhuis commented Aug 10, 2016

bnoordhuis commented Aug 23, 2016

eugeneo commented Aug 23, 2016

eugeneo commented Aug 23, 2016

bnoordhuis commented Sep 11, 2016

bnoordhuis commented Sep 11, 2016

eugeneo commented Sep 14, 2016

bnoordhuis commented Sep 14, 2016 •

edited

Loading

bnoordhuis commented Sep 23, 2016

bnoordhuis commented Sep 23, 2016

bnoordhuis commented Oct 3, 2016

mscdex commented Oct 6, 2016 •

edited

Loading

MylesBorins commented Oct 24, 2016

mscdex commented Oct 24, 2016

MylesBorins commented Oct 24, 2016

mscdex commented Oct 24, 2016

MylesBorins commented Nov 14, 2016

jbergstroem commented Nov 15, 2016

mscdex commented Nov 18, 2016

build: run cctests as part of test-ci target #8034

build: run cctests as part of test-ci target #8034

Conversation

bnoordhuis commented Aug 9, 2016 • edited Loading

bnoordhuis commented Aug 9, 2016

richardlau Aug 9, 2016

Choose a reason for hiding this comment

jbergstroem commented Aug 9, 2016

joaocgreis commented Aug 10, 2016

bnoordhuis commented Aug 10, 2016

bnoordhuis commented Aug 23, 2016

eugeneo commented Aug 23, 2016

eugeneo commented Aug 23, 2016

bnoordhuis commented Sep 11, 2016

bnoordhuis commented Sep 11, 2016

eugeneo commented Sep 14, 2016

bnoordhuis commented Sep 14, 2016 • edited Loading

bnoordhuis commented Sep 23, 2016

bnoordhuis commented Sep 23, 2016

bnoordhuis commented Oct 3, 2016

mscdex commented Oct 6, 2016 • edited Loading

MylesBorins commented Oct 24, 2016

mscdex commented Oct 24, 2016

MylesBorins commented Oct 24, 2016

mscdex commented Oct 24, 2016

MylesBorins commented Nov 14, 2016

jbergstroem commented Nov 15, 2016

mscdex commented Nov 18, 2016

bnoordhuis commented Aug 9, 2016 •

edited

Loading

bnoordhuis commented Sep 14, 2016 •

edited

Loading

mscdex commented Oct 6, 2016 •

edited

Loading