1273158 - Can't run desktop_unittest.py with --no-run-tests followed by --run-tests afterwards

Status: RESOLVED WONTFIX

(Release Engineering :: Applications: MozharnessCore, defect, P2)

Description

[Andrew Halberstadt [:ahal]]

Mozharness' concept of actions is supposed to make the scripts modular. I.e, we should be able to omit certain actions and run them later if we want.

In bug 1250904, we want to be able to set up the job but not actually run any tests. On the surface, this should be easy.. first run desktop_unittest.py with --no-run-tests, and at a later time, run it with --run-tests.

However, the script depends on a lot state set on the class instance. This obviously is not preserved between mozharness invocations. Currently, trying to run --run-tests after the fact results in:

Uncaught exception: Traceback (most recent call last):
  File "/home/worker/workspace/mozharness/mozharness/base/script.py", line 1768, in run
    self.run_action(action)
  File "/home/worker/workspace/mozharness/mozharness/base/script.py", line 1707, in run_action
    self._possibly_run_method(method_name, error_if_missing=True)
  File "/home/worker/workspace/mozharness/mozharness/base/script.py", line 1647, in _possibly_run_method
    return getattr(self, method_name)()
  File "/home/worker/workspace/mozharness/scripts/desktop_unittest.py", line 475, in run_tests
    preflight_run_method=self.preflight_xpcshell)
  File "/home/worker/workspace/mozharness/scripts/desktop_unittest.py", line 562, in _run_category_suites
    preflight_run_method(suites)
  File "/home/worker/workspace/mozharness/scripts/desktop_unittest.py", line 516, in preflight_xpcshell
    self.preflight_copydirs(self.config['xpcshell_name'])
  File "/home/worker/workspace/mozharness/scripts/desktop_unittest.py", line 500, in preflight_copydirs
    os.path.join(abs_app_dir, bin_name))
  File "/usr/lib/python2.7/shutil.py", line 128, in copy2
    copyfile(src, dst)
  File "/usr/lib/python2.7/shutil.py", line 83, in copyfile
    with open(dst, 'wb') as fdst:
IOError: [Errno 2] No such file or directory: '/home/worker/build/firefox/xpcshell'
Running post_fatal callback...
Exiting -1

I imagine there will be other failures too after this one gets fixed related to state not being set. I also imagine other mozharness scripts suffer from a similar problem.

A takeaway is we should avoid relying on shared state between actions. We should try to make each action its own independent island. This is likely easier said than done.

Comment 1

[Armen [:armenzg]]

Yes, Mozharness has always had this issue.
I had thought in the past to check if tests and binaries are already on disk but that required making assumptions of where they lay on disk.

Comment 2

[Andrew Halberstadt [:ahal]]

I've been thinking about this, and I think we need a persistent writeable config (i.e changes stored on disk). I know immutable configs were always part of mozharness' design goals, but I feel we just work around that immutability by copying config values to variables and then mutating those instead.

I think I'd like mozharness scripts to load configs as follows (with later overwriting earlier):
1) Apply config files
2) Apply persistent config store (if it exists)
3) Apply command line configs

Now we can either make the main config mutable and automatically have changes write out to the persistent storage.. or we could leave it immutable and have a separate API that writes out to storage. Or if we want to be really crazy, instance variables on the class could automatically write out to storage on change.

Comment 3

[Andrew Halberstadt [:ahal]]

I think comment 2 is the general purpose solution to this problem in mozharness. For the purposes of this bug, we could hack probably hack something in.. though I'd prefer to do it properly as long as there is no opposition.

Comment 4

[Andrew Halberstadt [:ahal]]

Actually, I don't think this blocks bug 1250904, it's more of a blocker on bug 1262260. Even then it's more of a nice-to-have.

What I want to support more than this is to run mozharness with --no-run-tests, and then use mach (in the tests.zip) to go from there. Still, this is something we should tackle as well.

Comment 5

[Jordan Lund (:jlund)]

(In reply to Andrew Halberstadt [:ahal] from comment #2)
> I've been thinking about this, and I think we need a persistent writeable
> config (i.e changes stored on disk). I know immutable configs were always
> part of mozharness' design goals, but I feel we just work around that
> immutability by copying config values to variables and then mutating those
> instead.
> 
> I think I'd like mozharness scripts to load configs as follows (with later
> overwriting earlier):
> 1) Apply config files
> 2) Apply persistent config store (if it exists)
> 3) Apply command line configs
> 

sounds similar to https://bugzilla.mozilla.org/show_bug.cgi?id=1007280

gmta :)

Comment 6

[Andrew Halberstadt [:ahal]]

Haha, yes, that's pretty much exactly what I was thinking of. I'm not necessarily hard blocking on that as we could still do something hacky here.. but it would certainly be nice.

Comment 7

[Armen [:armenzg]]

Marking as P2 as agreed on the meeting.

Comment 8

[Armen [:armenzg]]

Should we keep the dependency? or move it to "see also"?

Comment 9

[Andrew Halberstadt [:ahal]]

I'd keep the dependency, I doubt it's worth spending *too* much time on this, and the other bug makes this much much easier.

Comment 10

[Aki Sasaki (not active)]

Aiui mozharness has pretty much dropped its standalone actions mandate and we've steadily been moving functionality into mach and taskcluster. Do we still want to keep this bug open?

Comment 11

[Andrew Halberstadt [:ahal]]

Nope, let's WONTFIX.