Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Reconcile v0.10 with new CI #25686

Closed
wants to merge 10 commits into from
Closed

Reconcile v0.10 with new CI #25686

wants to merge 10 commits into from

Conversation

orangemocha
Copy link
Contributor

As part of CI reconciliation, these commits make v0.10 work with the jobs at https://jenkins-iojs.nodesource.com/

The first two commits (9ddc686 and 851e32c) are probably not strictly necessary, but they do a useful refactoring and they bring the v0.10 test runner closer to the v0.12 version.

The last commit (783cc5e) is an attempt to mark the current test failures in v0.10 as flaky (there a quite a few!). I will file issues to investigate those as soon as this PR lands. Some of these tests fail every time, so they are not actually flaky. A platform-specific skip might be more appropriate for those, but I defer that decision to the investigation that will follow, so that we can unblock node-accept-pull-request and CI reconciliation.

The other commits in this PR are pretty much a backport of #25653 to v0.10.

/cc @misterdjules @rvagg @jbergstroem

bnoordhuis and others added 2 commits July 15, 2015 02:03
test.py imports deps/v8/tools/utils.py but that file is gone after the
upgrade to 3.18.4 in commit 2f75785. Resurrect the file in tools/
@@ -1452,15 +1487,15 @@ def wrap(processor):

result = None
def DoSkip(case):
return SKIP in case.outcomes or SLOW in case.outcomes
return SKIP in case.outcomes or SLOW in case.outcomes or (FLAKY in case.outcomes and options.flaky_tests == "skip")

Choose a reason for hiding this comment

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

Styling: can we put the added condition on another line to avoid that long line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@misterdjules
Copy link

Thank you for doing that 👍 Ran make test in my local v0.10 clone with these commits and all was fine.

So aside from the style nitpick and my question, LGTM!

@misterdjules
Copy link

@orangemocha Also, if you haven't used it in the past, remember to use https://www.npmjs.com/package/git-apply-pr to merge the change in v0.10.

@orangemocha
Copy link
Contributor Author

Also, if you haven't used it in the past, remember to use https://www.npmjs.com/package/git-apply-pr to merge the change in v0.10.

I can now use node-accept-pull-request to land this 😄

orangemocha and others added 7 commits July 16, 2015 20:33
Adding --flaky-tests option, to allow regarding flaky tests failures
as non-fatal.

Currently only observed by the TapProgressIndicator, which will
add a # TODO directive to tests classified as flaky. According to the
TAP specification, the test harness is supposed to treat failures
that have a # TODO directive as non-fatal.
This is a minimal effort to support test output written both to
stdout and file in order to get our buildbots understanding
test output.

Cherry picked from jbergstroem/node@3194073
Original commit message follows:
  PR-URL: nodejs/node#934
  Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	tools/test.py

Conflicts:
	tools/test.py
On a few of our installations (namely CentOS), passing 'INFO'
resulted in a silent loglevel. Use a logging constant instead.

Cherry-picked from nodejs/node@8606793
Original commit metadata follows:
  Fixes: nodejs/build#104
  PR-URL: nodejs/node#1842
  Reviewed-By: Rod Vagg <rod@vagg.org>
Adding support for specifying flaky test mode to
the test runner:
- via an environment variable FLAKY_TESTS for Makefile
- via an argument ignore-flaky for vcbuild.bat
Make the test runner return a 0 exit code when only
flaky tests fail and --flaky-tests=dontcare is specified.
Adding a single rule to be called from Jenkins.
@orangemocha
Copy link
Contributor Author

Fixed according to feedback.

@misterdjules
Copy link

@orangemocha

Also, if you haven't used it in the past, remember to use https://www.npmjs.com/package/git-apply-pr to merge the change in v0.10.

I can now use node-accept-pull-request to land this

Ah great, I thought some of the actually failing and non-flaky tests had been left out of the list of flaky tests, but it makes sense to mark them as flaky. Excellent 👍

orangemocha pushed a commit that referenced this pull request Jul 17, 2015
test.py imports deps/v8/tools/utils.py but that file is gone after the
upgrade to 3.18.4 in commit 2f75785. Resurrect the file in tools/

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha pushed a commit that referenced this pull request Jul 17, 2015
PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
Adding --flaky-tests option, to allow regarding flaky tests failures
as non-fatal.

Currently only observed by the TapProgressIndicator, which will
add a # TODO directive to tests classified as flaky. According to the
TAP specification, the test harness is supposed to treat failures
that have a # TODO directive as non-fatal.

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
This is a minimal effort to support test output written both to
stdout and file in order to get our buildbots understanding
test output.

Cherry picked from jbergstroem/node@3194073
Original commit message follows:
  PR-URL: nodejs/node#934
  Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	tools/test.py

Conflicts:
	tools/test.py

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha pushed a commit that referenced this pull request Jul 17, 2015
On a few of our installations (namely CentOS), passing 'INFO'
resulted in a silent loglevel. Use a logging constant instead.

Cherry-picked from nodejs/node@8606793
Original commit metadata follows:
  Fixes: nodejs/build#104
  PR-URL: nodejs/node#1842
  Reviewed-By: Rod Vagg <rod@vagg.org>

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
Adding support for specifying flaky test mode to
the test runner:
- via an environment variable FLAKY_TESTS for Makefile
- via an argument ignore-flaky for vcbuild.bat

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
Make the test runner return a 0 exit code when only
flaky tests fail and --flaky-tests=dontcare is specified.

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
Adding a single rule to be called from Jenkins.

PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
orangemocha added a commit that referenced this pull request Jul 17, 2015
PR-URL: #25686
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
@orangemocha
Copy link
Contributor Author

Landed in a2f879f and adjacent commits.

Merge courtesy of https://jenkins-iojs.nodesource.com/job/node-accept-pull-request/21/ 😄

@orangemocha
Copy link
Contributor Author

.. and filed issues for all the tests marked as flaky, as you might have seen in your inbox (or spam folder 😉 )

@misterdjules
Copy link

@orangemocha Thank you for taking the time to do that 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants