build: run cctests as part of test-ci target by bnoordhuis · Pull Request #8034 · nodejs/node · GitHub

For discussion.

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

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/

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 ....

cc @nodejs/build

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

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

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

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
.

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

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

@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

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

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>

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

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

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

@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>

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

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

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

@thealphanerd #8948

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

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>

@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

build: run cctests as part of test-ci target by bnoordhuis · Pull Request #8034 · nodejs/node · GitHub

Uh oh!

Uh oh!

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

build: run cctests as part of test-ci target#8034bnoordhuis merged 3 commits intonodejs:masternodejs/node:masterfrom bnoordhuis:gtest-tapbnoordhuis/io.js:gtest-tapCopy head branch name to clipboard

Conversation

bnoordhuis commented Aug 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnoordhuis commented Aug 9, 2016

Uh oh!

richardlau Aug 9, 2016

Choose a reason for hiding this comment

Uh oh!

jbergstroem commented Aug 9, 2016

Uh oh!

joaocgreis commented Aug 10, 2016

Uh oh!

bnoordhuis commented Aug 10, 2016

Uh oh!

bnoordhuis commented Aug 23, 2016

Uh oh!

eugeneo commented Aug 23, 2016

Uh oh!

eugeneo commented Aug 23, 2016

Uh oh!

bnoordhuis commented Sep 11, 2016

Uh oh!

bnoordhuis commented Sep 11, 2016

Uh oh!

eugeneo commented Sep 14, 2016

Uh oh!

bnoordhuis commented Sep 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnoordhuis commented Sep 23, 2016

Uh oh!

bnoordhuis commented Sep 23, 2016

Uh oh!

bnoordhuis commented Oct 3, 2016

Uh oh!

mscdex commented Oct 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MylesBorins commented Oct 24, 2016

Uh oh!

mscdex commented Oct 24, 2016

Uh oh!

MylesBorins commented Oct 24, 2016

Uh oh!

mscdex commented Oct 24, 2016

Uh oh!

MylesBorins commented Nov 14, 2016

Uh oh!

jbergstroem commented Nov 15, 2016

Uh oh!

mscdex commented Nov 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

build: run cctests as part of test-ci target#8034
bnoordhuis merged 3 commits into
nodejs:masterfrom
bnoordhuis:gtest-tap

bnoordhuis commented Aug 9, 2016 •
edited
Loading

bnoordhuis commented Sep 14, 2016 •
edited
Loading

mscdex commented Oct 6, 2016 •
edited
Loading