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

Exit wptrunner with an error status if no tests ran #10030

Merged
merged 2 commits into from Mar 14, 2018

Conversation

Projects
None yet
5 participants
@jgraham
Copy link
Contributor

jgraham commented Mar 14, 2018

Typically no tests completing means that some error occured, or the user
supplied an invalid test path. In either case we should log the
problem and return a failure code.


This change is Reviewable

@wpt-pr-bot wpt-pr-bot added the infra label Mar 14, 2018

@wpt-pr-bot wpt-pr-bot requested a review from gsnedders Mar 14, 2018

@jgraham

This comment has been minimized.

Copy link
Contributor Author

jgraham commented Mar 14, 2018

This partially addresses #9979

@Ms2ger

Ms2ger approved these changes Mar 14, 2018

return unexpected_total == 0
if test_total == 0:
logger.error("No tests ran")
return test_total > 0 and unexpected_total == 0

This comment has been minimized.

Copy link
@Ms2ger

Ms2ger Mar 14, 2018

Contributor

Nit:

    if test_total == 0:
        logger.error("No tests ran")
        return False

    return unexpected_total == 0
def test_count(self):
return sum(item.test_count for item in self.pool)

def subtest_count(self):

This comment has been minimized.

Copy link
@Ms2ger

Ms2ger Mar 14, 2018

Contributor

Unused?

This comment has been minimized.

Copy link
@gsnedders

gsnedders Mar 14, 2018

Contributor

Yeah, this seems dead.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 14, 2018

Build PASSED

Started: 2018-03-14 17:24:28
Finished: 2018-03-14 17:40:30

View more information about this build on:

@jgraham jgraham force-pushed the wptrunner_exit_error branch from c5719e0 to 0099cae Mar 14, 2018

@jgraham

This comment has been minimized.

Copy link
Contributor Author

jgraham commented Mar 14, 2018

Of course landing this is hard until the underlying issue from #9979 is fixed.

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Mar 14, 2018

Should we just drop Chrome given #9979 so we can land this (and ensure Firefox doesn't regress), given that's not actually regressing testing Chrome? I'd be okay with that, and then #9979 becomes "re-enable Chrome" instead of "Chrome runs nothing".

<script>
var timeout_multiplier = 1;
var win = null;
</script>

This comment has been minimized.

Copy link
@gsnedders

gsnedders Mar 14, 2018

Contributor

This seems unrelated to this commit.

def test_count(self):
return sum(item.test_count for item in self.pool)

def subtest_count(self):

This comment has been minimized.

Copy link
@gsnedders

gsnedders Mar 14, 2018

Contributor

Yeah, this seems dead.

jgraham added some commits Mar 14, 2018

Exit wptrunner with an error status if no tests ran
Typically no tests completing means that some error occured, or the user
supplied an invalid test path. In either case we should log the
problem and return a failure code.
Stop running Chrome wptrunner infrastructure tests for bustage
The tests were failing with some kind of Chromedriver/Chrome version
mismatch, so we can't block on these tests until that issue is fixed.

@jgraham jgraham force-pushed the wptrunner_exit_error branch from 0044c90 to 53471db Mar 14, 2018

@jgraham

This comment has been minimized.

Copy link
Contributor Author

jgraham commented Mar 14, 2018

OK, I didn't mind the subtest count existing even though it's not yet used.

Reverted the inadvertent change.

@jgraham jgraham merged commit 17dd2c4 into master Mar 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gsnedders gsnedders deleted the wptrunner_exit_error branch Mar 14, 2018

Hexcles added a commit that referenced this pull request Mar 19, 2018

Exit check_stability with an error when no tests run
Similar to #10030 (which was for `wpt run`), this commit makes `wpt
check-stability` return a non-zero exit code when no tests run.

Also fix some code smells:

* `run` was redefined by `do_delayed_imports` to a different function.
  Rename the other `run` method in this module to `setup_and_run`.
* `retcode` was unnecessarily defined in the top scope. Remove it to
  avoid redefinition.
* `global logger` and the call to `get_parser` were extraneous in `run`
  (now `setup_and_run`) and hence are removed.

Hexcles added a commit that referenced this pull request Mar 20, 2018

Exit check_stability with an error when no tests run
Similar to #10030 (which was for `wpt run`), this commit makes `wpt
check-stability` return a non-zero exit code when no tests run.

Also fix some code smells:

* `run` was redefined by `do_delayed_imports` to a different function.
  Rename the other `run` method in this module to `setup_and_run`.
* `retcode` was unnecessarily defined in the top scope. Remove it to
  avoid redefinition.
* `global logger` and the call to `get_parser` were extraneous in `run`
  (now `setup_and_run`) and hence are removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.