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

NEW
Unassigned

Status

Release Engineering
Mozharness
P2
normal
2 years ago
a year ago

People

(Reporter: ahal, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 years ago
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

2 years ago
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.
(Reporter)

Comment 2

2 years ago
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.
(Reporter)

Comment 3

2 years ago
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.
(Reporter)

Comment 4

2 years ago
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.
Blocks: 1262260
No longer blocks: 1250904

Comment 5

2 years ago
(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 :)
(Reporter)

Comment 6

2 years ago
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.
Depends on: 1007280

Comment 7

2 years ago
Marking as P2 as agreed on the meeting.
Priority: -- → P2

Comment 8

2 years ago
Should we keep the dependency? or move it to "see also"?
(Reporter)

Comment 9

a year ago
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.
Assignee: ahalberstadt → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.