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

Reconcile v0.12 with new CI #25653

Closed
wants to merge 6 commits into from
Closed

Reconcile v0.12 with new CI #25653

wants to merge 6 commits into from

Conversation

orangemocha
Copy link
Contributor

..continued from #25541

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

This PR addresses all review feedback from #25541. In particular:

  • I left out the commit that changed the default for snapshots, based on feedback from @misterdjules . Instead, I introduced a new run-ci makefile rule which invokes configure internally with --without-snapshot. On Windows, the same is accomplished by calling vcbuild test-ci
  • I made it so that Jenkins jobs can control whether to care about flaky tests or not, with an environment variable on *nix and a vcbuild argument on Windows.

orangemocha and others added 2 commits July 8, 2015 20:01
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 <[email protected]>
  Reviewed-By: Ben Noordhuis <[email protected]>

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 <[email protected]>
@@ -81,7 +84,6 @@ goto next-arg

:args-done
if defined upload goto upload
if defined jslint goto jslint

Choose a reason for hiding this comment

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

Does vcbuild jslint still work? It seems that with this change it would only work when tests are run too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. This change enables building and testing, when jslint is specified. But it has the side effect of not getting to jslint unless you run tests. Will fix.

@misterdjules
Copy link

Thank you @orangemocha! Added a few comments.

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

Conflicts:
	Makefile
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

Updated to fix extra line and jslint issue.

Sneak preview CI run: https://jenkins-iojs.nodesource.com/job/orangemocha-test-commit/4/

@misterdjules
Copy link

LGTM. That's great! Do we plan to backport this to v0.10 too and test + build v0.10 releases from the converged CI?

@orangemocha
Copy link
Contributor Author

I might give it a shot, but given that we currently don't use node-accept-pull-request for v0.10, I will probably settle for just porting the current workflow, and then work on enabling node-accept-pull-request on v0.10 later.

What job do you currently use on v0.10, BTW?

Thanks for reviewing!

orangemocha added a commit that referenced this pull request Jul 10, 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 <[email protected]>
  Reviewed-By: Ben Noordhuis <[email protected]>

Conflicts:
	tools/test.py

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: #25653
orangemocha pushed a commit that referenced this pull request Jul 10, 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 <[email protected]>

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: #25653
orangemocha added a commit that referenced this pull request Jul 10, 2015
Reviewed-By: Julien Gilli <[email protected]>
PR-URL: #25653
orangemocha added a commit that referenced this pull request Jul 10, 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

Conflicts:
	Makefile

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: #25653
orangemocha added a commit that referenced this pull request Jul 10, 2015
Make the test runner return a 0 exit code when only
flaky tests fail and --flaky-tests=dontcare is specified.

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: #25653
orangemocha added a commit that referenced this pull request Jul 10, 2015
Adding a single rule to be called from Jenkins.

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: #25653
@orangemocha
Copy link
Contributor Author

Landed in c9ef4b3.

@misterdjules
Copy link

@orangemocha Sounds good! Currently we use the node-review job to test changes targeted to v0.10, and we land them with git-apply-pr. Ideally we would use node-accept-pull-request and mark currently v0.10 flaky tests as such, and create issues to fix them.

jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
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 <[email protected]>
  Reviewed-By: Ben Noordhuis <[email protected]>

Conflicts:
	tools/test.py

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: nodejs#25653
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
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 <[email protected]>

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: nodejs#25653
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
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

Conflicts:
	Makefile

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: nodejs#25653
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Make the test runner return a 0 exit code when only
flaky tests fail and --flaky-tests=dontcare is specified.

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: nodejs#25653
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
Adding a single rule to be called from Jenkins.

Reviewed-By: Julien Gilli <[email protected]>
PR-URL: nodejs#25653
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.

3 participants