Run the hazard analysis self-test as part of the hazard jobs

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P2
normal
RESOLVED FIXED
Last year
6 months ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(4 attachments)

I have a nice set of tests. I probably ought to run them.

I'm going to stick some of my other hazard job cleanup patches in this bug.
Ed Morley requested this in 2013. But he's not accepting review requests at the moment. It seems to work, though:

https://treeherder.mozilla.org/#/jobs?repo=try&author=sfink@mozilla.com&fromchange=fad6a21ac0ae6537f4f31e1a5d8c192294efcd02&selectedJob=191905359
Attachment #8997569 - Flags: review?(jcoppeard)
Perhaps not the right bug, but this was failing and it would have helped to see the actual error.
Attachment #8997570 - Flags: review?(jcoppeard)
I don't understand why, but my local run of the taskcluster hazard job worked without this, but the actual production one failed.

I need to roll this into the job addition before landing.
Attachment #8997573 - Flags: review?(jcoppeard)
Attachment #8997559 - Flags: review?(jcoppeard) → review+
Comment on attachment 8997569 [details] [diff] [review]
Format errors better for taskcluster

Review of attachment 8997569 [details] [diff] [review]:
-----------------------------------------------------------------

Oh, nice.

::: js/src/devtools/rootAnalysis/run-test.py
@@ +93,5 @@
>      test = Test(indir, outdir, cfg, verbose=cfg.verbose)
>  
>      os.chdir(outdir)
> +    if glob("*.xdb"):
> +        subprocess.call(["sh", "-c", "rm *.xdb"])

If we're going to glob() in python anyway we should probably unlink the files from python too to avoid "rm *" in the shell.
Attachment #8997569 - Flags: review?(jcoppeard) → review+
Comment on attachment 8997570 [details] [diff] [review]
Report errors creating directory (other than File exists)

Review of attachment 8997570 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/devtools/rootAnalysis/run-test.py
@@ +79,5 @@
> +def make_dir(dirname, exist_ok=True):
> +    try:
> +        os.mkdir(dirname)
> +    except OSError as e:
> +        if exist_ok and e.strerror == 'File exists':

Checking the error seems fragile.  Could we check os.path.isdir() first instead?
Attachment #8997570 - Flags: review?(jcoppeard) → review+
Attachment #8997573 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #5)
> If we're going to glob() in python anyway we should probably unlink the
> files from python too to avoid "rm *" in the shell.

Good point, I'll do that.

(In reply to Jon Coppeard (:jonco) from comment #6)
> Checking the error seems fragile.  Could we check os.path.isdir() first
> instead?

Yes, and I do that other places, though I feel a little dirty when I do -- it introduces a race condition between the check and the create. I'm not sure which one is "cleaner".
(In reply to Steve Fink [:sfink] [:s:] from comment #7)

> Yes, and I do that other places, though I feel a little dirty when I do --
> it introduces a race condition between the check and the create.

Indeed.

Ah, you're using python3 and happily it seems os.mkdirs has now has an exist_ok argument that will handle this.
Priority: -- → P2
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c5c021b7271
Format errors better for taskcluster, r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/c45f9de01d9e
Run the hazard analysis self-test as part of the hazard jobs, r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a02fd73f286
Report errors creating directory (other than File exists), r=jonco
https://hg.mozilla.org/mozilla-central/rev/1c5c021b7271
https://hg.mozilla.org/mozilla-central/rev/c45f9de01d9e
https://hg.mozilla.org/mozilla-central/rev/3a02fd73f286
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Duplicate of this bug: 1008308
You need to log in before you can comment on or make changes to this bug.