Skip to content

build: move doc tests to separate Travis job#26992

Closed
richardlau wants to merge 2 commits intonodejs:masterfrom
richardlau:travis-split
Closed

build: move doc tests to separate Travis job#26992
richardlau wants to merge 2 commits intonodejs:masterfrom
richardlau:travis-split

Conversation

@richardlau
Copy link
Member

@richardlau richardlau commented Mar 29, 2019

Move the doc tests to a separate Travis job to reduce the overall time for the Test Suite job. This should (hopefully) bring the time for the Test Suite job back under the 50 minute time limit Travis imposes.

The test target currently differs from test-only in that test-only doesn't run test-docs:

node/Makefile

Lines 288 to 296 in 415a825

.PHONY: test
# This does not run tests of third-party libraries inside deps.
test: all ## Runs default tests, linters, and builds docs.
$(MAKE) -s test-doc
$(MAKE) -s build-addons
$(MAKE) -s build-js-native-api-tests
$(MAKE) -s build-node-api-tests
$(MAKE) -s cctest
$(MAKE) -s jstest

node/Makefile

Lines 298 to 304 in 415a825

.PHONY: test-only
test-only: all ## For a quick test, does not run linter or build docs.
$(MAKE) build-addons
$(MAKE) build-js-native-api-tests
$(MAKE) build-node-api-tests
$(MAKE) cctest
$(MAKE) jstest

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 29, 2019

@richardlau Sadly, an error occurred when I tried to trigger a build. :(

@richardlau richardlau added the build Issues and PRs related to build files or the CI. label Mar 29, 2019
@richardlau

This comment has been minimized.

Sign in to view
@richardlau richardlau mentioned this pull request Mar 29, 2019
Closed
@richardlau

This comment has been minimized.

Sign in to view
@richardlau
build: move doc tests to separate Travis job
2bf1364 
Move the doc tests to a separate Travis job to reduce the overall
time for the `Test Suite` job.
@lpinca
Copy link
Member

lpinca commented Mar 29, 2019

Not enought, it seems :(

@richardlau
Copy link
Member Author

richardlau commented Mar 29, 2019

Not enought, it seems :(

Yeah... At least the Documentation job completed (and populated the ccache). I've restarted the timed out Test Suite job which I expect will now complete as the PR has a ccache it can use from the Documentation job. If it does I'll try implementing build stages and order the Test Suite job to run last (at the moment all jobs runs in parallel).

@richardlau
Copy link
Member Author

richardlau commented Mar 30, 2019

I've implemented stages and wiped the ccache for this PR. We'll see what Travis makes of it 🤞.

@richardlau richardlau mentioned this pull request Mar 30, 2019
Closed
2 tasks
@richardlau
Copy link
Member Author

richardlau commented Mar 30, 2019

I've implemented stages and wiped the ccache for this PR. We'll see what Travis makes of it 🤞.

Build passed so the theory is sound. We don't actually need to split the doc tests out if we use staging. Rather than cause confusion I've opened a new PR (#27002) based on what this evolved into with some tweaks (i.e. no longer splitting out the doc tests).

@richardlau richardlau closed this Mar 30, 2019
@richardlau richardlau deleted the travis-split branch March 30, 2019 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes

+1 more reviewer

@refack refack refack approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@richardlau @nodejs-github-bot @lpinca @refack