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

InspectorTest regression on mac #8006

Closed
ofrobots opened this issue Aug 7, 2016 · 9 comments
Closed

InspectorTest regression on mac #8006

ofrobots opened this issue Aug 7, 2016 · 9 comments
Labels
inspector Issues and PRs related to the V8 inspector protocol macos Issues and PRs related to the macOS platform / OSX.

Comments

@ofrobots
Copy link
Contributor

ofrobots commented Aug 7, 2016

0190db4 regressed InspectorTest on mac. See #8002.

There are two problems here:

  • Failure needs to be investigated.
  • The CI missed this.

/cc @nodejs/v8-inspector @bnoordhuis @nodejs/build.

@addaleax addaleax added macos Issues and PRs related to the macOS platform / OSX. inspector Issues and PRs related to the V8 inspector protocol labels Aug 7, 2016
@Trott
Copy link
Member

Trott commented Aug 8, 2016

The CI missed this.

Just a guess, but maybe CI missed it because there was a four day gap between the CI run and when the commit was landed. So maybe some other change landed during that time. And that change by itself is fine. And this change by itself is fine. But the two changes together mean failing test.

That's the first explanation that leaps to mind, at least...

@Trott
Copy link
Member

Trott commented Aug 8, 2016

Oh, wait, if CI is still missing this than my speculation isn't the issue. Sorry for the noise.

@bnoordhuis
Copy link
Member

FWIW, I could initially reproduce with a build from last week but after updating to today's HEAD of master and rebuilding, cctest passes again.

@jbergstroem
Copy link
Member

OS X version discrepancy in CI vs local perhaps?

@addaleax
Copy link
Member

addaleax commented Aug 8, 2016

Probably I just don’t know enough about the magic behind the CI, but does cctest even run on CI? I can’t find anything that looks like it in the console output.

@bnoordhuis
Copy link
Member

It doesn't seem to be part of the test-ci target. For that matter, gtest doesn't support tap output, AFAIK.

@ofrobots
Copy link
Contributor Author

ofrobots commented Aug 8, 2016

Ah. that explains why the CI missed it. I can still reproduce the issue with the latest master. I believe @eugeneo is planning to investigate the failure itself.

@jbergstroem
Copy link
Member

@bnoordhuis: Correct. We could look at parsing xunit output through a plugin? The difference between xunit and junit (what Jenkins natively prefers) seems pretty small. Stackoverflow suggests awk:ing it which we also could give a go.

@eugeneo
Copy link
Contributor

eugeneo commented Aug 8, 2016

I'm looking into the test failure.

On Mon, Aug 8, 2016 at 8:49 AM Johan Bergström notifications@github.com
wrote:

@bnoordhuis https://github.com/bnoordhuis: Correct. We could look at
parsing xunit output through a plugin? The difference between xunit and
junit (what Jenkins natively prefers) seems pretty small. Stackoverflow
suggests awk:ing it which we also could give a go.


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

cjihrig pushed a commit that referenced this issue Aug 10, 2016
Test was updated to wait till the inspector processes socket closure.

Fixes: #8006
PR-URL: #8019
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Development

No branches or pull requests

6 participants