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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make wptrun fail correctly when certutil not found #6297

Merged
merged 1 commit into from
Jun 21, 2017

Conversation

sideshowbarker
Copy link
Contributor

@sideshowbarker sideshowbarker commented Jun 21, 2017

This change makes wptrun correctly handle the case where it can’t find
the certutil binary.

Without this change you end up with the value of path being None
when it reaches this part of the tools/browserutils/browser.py code:

if os.path.splitdrive(path)[1].split(os.path.sep) == ["", "Windows", "system32", "certutil.exe"]:

... which results in:

AttributeError: 'NoneType' object has no attribute 'split'

So this change just early-returns None from there if path is None.


This change is Reviewable

This change makes wptrun correctly handle the case where it can’t find
the certutil binary.

Without this change you end up with the value of `path` being `None`
when it reaches this part of the tools/browserutils/browser.py code:

    if os.path.splitdrive(path)[1].split(os.path.sep) == ["", "Windows", "system32", "certutil.exe"]:

... which results in:

    AttributeError: 'NoneType' object has no attribute 'split'

So this change just early-returns `None` from there if `path` is `None`.
@ghost
Copy link

ghost commented Jun 21, 2017

View the complete job log.

Firefox (nightly)

Testing web-platform-tests at revision 030bba7
Using browser at version BuildID 20170612100241; SourceStamp 27cad9749cddf68e11fdd4e5d73dad84a8f8cf23
Starting 10 test iterations
No tests run.

@ghost
Copy link

ghost commented Jun 21, 2017

View the complete job log.

Sauce (safari)

Testing web-platform-tests at revision 030bba7
Using browser at version 10.0
Starting 10 test iterations
No tests run.

@ghost
Copy link

ghost commented Jun 21, 2017

View the complete job log.

Chrome (unstable)

Testing web-platform-tests at revision 030bba7
Using browser at version 61.0.3135.4 dev
Starting 10 test iterations
No tests run.

@ghost
Copy link

ghost commented Jun 21, 2017

View the complete job log.

Sauce (MicrosoftEdge)

Testing web-platform-tests at revision 030bba7
Using browser at version 14.14393
Starting 10 test iterations
No tests run.

@jgraham
Copy link
Contributor

jgraham commented Jun 21, 2017

Reviewed 1 of 1 files at r1.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@jgraham jgraham merged commit 9fa3615 into master Jun 21, 2017
@jgraham jgraham deleted the sideshowbarker/wptrun-certutil-none branch June 21, 2017 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants