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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: run node tests on CI #19403

Merged
merged 6 commits into from Jul 24, 2019

Conversation

@MarshallOfSound
Copy link
Member

MarshallOfSound commented Jul 23, 2019

This PR does a few things

  • Runs the node.js test suite on our CI 馃帀
  • Ignores all the node.js tests that are currently failing (around 300 / 3000)
  • Fixes an issue where ELECTRON_RUN_AS_NODE would not parse CLI flags correctly because the v8 isolate was initialized before SetFlagsFromCommandLine was called inside node::Init. A quick re-ordering fixed that issue 馃憤

Notes: ELECTRON_RUN_AS_NODE now correctly parses both node options and v8 flags. E.g. --expose_gc

@MarshallOfSound MarshallOfSound requested a review from electron/wg-upgrades as a code owner Jul 23, 2019
script/node-spec-runner.js Show resolved Hide resolved
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <sattard@slack-corp.com>
Date: Tue, 23 Jul 2019 11:36:48 -0700
Subject: chore: handle default_configuration not being set in the electron env

This comment has been minimized.

Copy link
@nornagon

nornagon Jul 23, 2019

Member

should we rather set this up in the env?

This comment has been minimized.

Copy link
@MarshallOfSound

MarshallOfSound Jul 23, 2019

Author Member

mmaayybbeeee but our config.gypi doesn't currently support dynamic changes, happy to raise a TODO though to update our GN files to generate the config.gypi 馃槅

Copy link
Contributor

jkleinsc left a comment

@MarshallOfSound do you know if the node tests support test output to file? If so we should save the test results to CircleCI:
https://circleci.com/docs/2.0/collect-test-data

@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

MarshallOfSound commented Jul 23, 2019

@jkleinsc I did some tricks and now node failures are reported through Circle

image

This re-orders our node clean up so that we free the environment before
the task runner is cleaned up as node uses the task runner during clean
up.  It also calls WaitForDisconnect() to ensure that inspector agents
are notified that the context is going down.
@MarshallOfSound MarshallOfSound force-pushed the node-tests branch from 5018934 to 5a1cc01 Jul 23, 2019
@MarshallOfSound MarshallOfSound force-pushed the node-tests branch from 5a1cc01 to 17cd76b Jul 23, 2019
@MarshallOfSound MarshallOfSound merged commit 6d83eaa into master Jul 24, 2019
13 checks passed
13 checks passed
Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190724.5 succeeded
Details
electron-arm64-testing Build #20190724.5 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented Jul 24, 2019

Release Notes Persisted

ELECTRON_RUN_AS_NODE now correctly parses both node options and v8 flags. E.g. --expose_gc

@MarshallOfSound MarshallOfSound deleted the node-tests branch Jul 24, 2019
@trop

This comment has been minimized.

Copy link
Contributor

trop bot commented Sep 3, 2019

A maintainer has manually backported this PR to "6-0-x", please check out #20098

@trop trop bot added the in-flight/6-0-x label Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.