Skip to content

tools: allow running test.py without configuring #16621

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 20, 2017
Merged

tools: allow running test.py without configuring #16621

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 9 additions & 19 deletions tools/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,8 +889,7 @@ class Context(object):

def __init__(self, workspace, buildspace, verbose, vm, args, expect_fail,
timeout, processor, suppress_dialogs,
store_unexpected_output, repeat, abort_on_timeout,
v8_enable_inspector):
store_unexpected_output, repeat, abort_on_timeout):
self.workspace = workspace
self.buildspace = buildspace
self.verbose = verbose
Expand All @@ -902,7 +901,7 @@ def __init__(self, workspace, buildspace, verbose, vm, args, expect_fail,
self.store_unexpected_output = store_unexpected_output
self.repeat = repeat
self.abort_on_timeout = abort_on_timeout
self.v8_enable_inspector = v8_enable_inspector
self.v8_enable_inspector = True

def GetVm(self, arch, mode):
if arch == 'none':
Expand Down Expand Up @@ -930,20 +929,6 @@ def RunTestCases(cases_to_run, progress, tasks, flaky_tests_mode):
progress = PROGRESS_INDICATORS[progress](cases_to_run, flaky_tests_mode)
return progress.Run(tasks)

def GetV8InspectorEnabledFlag():
# The following block reads config.gypi to extract the v8_enable_inspector
# value. This is done to check if the inspector is disabled in which case
# the '--inspect' flag cannot be passed to the node process as it will
# cause node to exit and report the test as failed. The use case
# is currently when Node is configured --without-ssl and the tests should
# still be runnable but skip any tests that require ssl (which includes the
# inspector related tests).
with open('config.gypi', 'r') as f:
s = f.read()
config_gypi = ast.literal_eval(s)
return config_gypi['variables']['v8_enable_inspector']


# -------------------------------------------
# --- T e s t C o n f i g u r a t i o n ---
# -------------------------------------------
Expand Down Expand Up @@ -1626,8 +1611,7 @@ def Main():
options.suppress_dialogs,
options.store_unexpected_output,
options.repeat,
options.abort_on_timeout,
GetV8InspectorEnabledFlag())
options.abort_on_timeout)

# Get status for tests
sections = [ ]
Expand Down Expand Up @@ -1670,6 +1654,12 @@ def Main():
all_cases += cases
all_unused.append(unused_rules)

# We want to skip the inspector tests if node was built without the inspector.
has_inspector = Execute([vm,
"-p", "process.config.variables.v8_enable_inspector"], context)
if has_inspector.stdout.rstrip() == "0":
context.v8_enable_inspector = False
Copy link
Member

Choose a reason for hiding this comment

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

Does this set context.v8_enable_inspector if process.config.variables.v8_enable_inspector is falsy?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only if it's specifically 0, not just generally falsy.

Copy link
Member

Choose a reason for hiding this comment

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

I guess this is okay as long as the only valid values for process.config.variables.v8_enable_inspector are 0 and non-zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT possible values are 0, 1 with undefined in v6 and v4.

Copy link
Contributor

Choose a reason for hiding this comment

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

For v6 it's process.config.variables.v8_inspector

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I'd be inclined to leave this as is here, and just not backport to 4.x and 6.x. Seem reasonable?

Copy link
Member

Choose a reason for hiding this comment

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

The V8 inspector support isn't in v4.x so no need to backport there.


if options.cat:
visited = set()
for test in unclassified_tests:
Expand Down