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

tools: explicit path for V8 test tap output #7460

Merged

Conversation

MylesBorins
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

Description of change

Currently we do not specific an absolute path for the tap output of the
V8 test suite. This is proving to be unreliable across release lines.

By prepending $(PWD) to each path we can guarantee it will always be
in the root folder.

This change will require an update to the jenkins job

@MylesBorins MylesBorins added v8 engine Issues and PRs related to the V8 dependency. tools Issues and PRs related to the tools directory. labels Jun 28, 2016
@MylesBorins
Copy link
Contributor Author

MylesBorins commented Jun 28, 2016

modified ci job: https://ci.nodejs.org/job/thealphanerd-test-commit-v8-linux/1/

edit: modified CI is green

/cc @mhdawson @nodejs/v8

@MylesBorins
Copy link
Contributor Author

/cc @nodejs/build not 100% this is the way we want to solve it... PTAL

@mhdawson
Copy link
Member

We could make it more flexible so you can specify the path, but I think this will be good for people running the test so I'm good with what you have

LGTM.

@jasnell
Copy link
Member

jasnell commented Jun 28, 2016

Lgtm

@thefourtheye
Copy link
Contributor

LGTM

Currently we do not specific an absolute path for the tap output of the
V8 test suite. This is proving to be unreliable across release lines.

By prepending `$(PWD)` to each path we can guarantee it will always be
in the root folder.

PR-URL: nodejs#7460
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@MylesBorins MylesBorins force-pushed the V8-testrunner-explicit-tap-path branch from 7b75e6b to cae428a Compare June 29, 2016 21:33
@MylesBorins MylesBorins merged commit cae428a into nodejs:master Jun 29, 2016
@MylesBorins
Copy link
Contributor Author

@mhdawson I've gone ahead and updated the CI job to no longer cp the file from v8 -> root as the new default is to place the xml in the root directory. This may need to be updated in other CI jobs outside of node.

MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Jun 29, 2016
Currently we do not specific an absolute path for the tap output of the
V8 test suite. This is proving to be unreliable across release lines.

By prepending `$(PWD)` to each path we can guarantee it will always be
in the root folder.

PR-URL: nodejs#7460
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
Currently we do not specific an absolute path for the tap output of the
V8 test suite. This is proving to be unreliable across release lines.

By prepending `$(PWD)` to each path we can guarantee it will always be
in the root folder.

PR-URL: #7460
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
@MylesBorins MylesBorins deleted the V8-testrunner-explicit-tap-path branch July 8, 2016 14:54
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Currently we do not specific an absolute path for the tap output of the
V8 test suite. This is proving to be unreliable across release lines.

By prepending `$(PWD)` to each path we can guarantee it will always be
in the root folder.

PR-URL: #7460
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants