stop_runner waits for the wrong thing

RESOLVED FIXED

Status

Testing Graveyard
Mozmill
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: Jeff Hammel, Assigned: Jeff Hammel)

Tracking

Details

(Whiteboard: [mozmill-2.0+])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
the stop_runner method demands that self.endRunnerCalled is true:

https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/__init__.py#L309

However, this is a weird bit of logic.  Since this is (currently) set
only by javascript, this is a pretty big assumption.  For instance the
following code does not work:

import jsbridge
import mozmill
import mozprofile
import mozrunner

# boilerplate
results = mozmill.TestResults('1.2.3')
profile_args = {'addons': [mozmill.extension_path,
jsbridge.extension_path]}
runner_args = {'cmdargs': ['-jsbridge', '24242', '-foreground']}
runner = mozrunner.create_runner(mozprofile.FirefoxProfile,
                                 mozrunner.FirefoxRunner,
                                 profile_args=profile_args,
                                 runner_args=runner_args)
# instantiate a MozMill
m = mozmill.MozMill(runner, results)

# run
m.run_tests([])

What should happen is....nothing ;).  What actually happens is, after
waiting too long, you get:

(mozmill)│python test_startstop.py 
Traceback (most recent call last):
  File "test_startstop.py", line 18, in <module>
    m.run_tests([])
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 239, in run_tests
    self.stop_runner()
  File
  "/home/jhammel/mozmill/src/mozmill/mozmill/mozmill/__init__.py",
  line 314, in stop_runner
    raise Exception('endRunner was never called. There must have been
    a failure in the framework')
Exception: endRunner was never called. There must have been a failure
in the framework

This severely limits our flexibility of using stop_runner.   I wanted
to utilize this for bug 638989 when I uncovered this.  While I can
work around this -- for now, at least -- for this particular problem,
in general asserting that the JS does something particular does not
make for the flexible architecture.
(Assignee)

Updated

7 years ago
Whiteboard: [mozmill-2.0?]

Updated

7 years ago
Whiteboard: [mozmill-2.0?] → [mozmill-2.0+]
(Assignee)

Updated

7 years ago
Blocks: 584470
(Assignee)

Updated

7 years ago
Blocks: 627160
(Assignee)

Updated

7 years ago
Depends on: 640010
(Assignee)

Comment 2

7 years ago
Created attachment 544921 [details] [diff] [review]
WIP
(Assignee)

Updated

7 years ago
Assignee: nobody → jhammel
(Assignee)

Comment 3

7 years ago
Created attachment 545275 [details] [diff] [review]
use the is_running method (and fix it)
Attachment #544921 - Attachment is obsolete: true
Attachment #545275 - Flags: review?(fayearthur+bugs)
(In reply to comment #3)
> Created attachment 545275 [details] [diff] [review] [review]
> use the is_running method (and fix it)

It doesn't look like you changed is_running(),  just wondering because you said it would "fix it"
(Assignee)

Comment 5

7 years ago
(In reply to comment #4)
> (In reply to comment #3)
> > Created attachment 545275 [details] [diff] [review] [review] [review]
> > use the is_running method (and fix it)
> 
> It doesn't look like you changed is_running(),  just wondering because you
> said it would "fix it"

I just added self to the calling signature; otherwise, bad things happen
Comment on attachment 545275 [details] [diff] [review]
use the is_running method (and fix it)

looks good if you've tested.
Attachment #545275 - Flags: review?(fayearthur+bugs) → review+
(Assignee)

Comment 7

7 years ago
Tested as best I can by hand. Somewhat hard-pressed to write tests for this sort of thing, though we should have more mozprocess tests kinda soonish
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.