809729 - "mach reftest" doesn't play well wtih EXTRA_TEST_ARGS=--debugger=gdb

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Cameron McCormack (:heycam) (away Jan 26 - Feb 1)]

Since mach grabs the output of running reftests, gdb doesn't a terminal that you can interact with it nicely when using EXTRA_TEST_ARGS=--debugger=gdb.

Comment 1

[Cameron McCormack (:heycam) (away Jan 26 - Feb 1)]

I suppose anyway that we don't want to support EXTRA_TEST_ARGS but have arguments that get passed to mach directly, like `./mach reftest --debugger=gdb`.

Comment 2

[:ehsan akhgari]

(Bug 797101 was filed for this capability under mochitests.)

Comment 3

[Gregory Szorc [:gps]]

Attachment: Add --debugger option to run reftests, v1

This is essentially the same patch as bug 797101. I also performed some basic cleanup so we don't see a stack trace if make exits with a non-0 exit code.

I was unable to run reftests on my tree when I wrote this because my tree wasn't built. However, I was able to see a gdb prompt with --debugger. I would appreciate verification that this works correctly.

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 688603 [details] [diff] [review]
Add --debugger option to run reftests, v1

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

::: layout/tools/reftest/mach_commands.py
@@ +93,5 @@
> +            pass_thru = True
> +
> +        if extra_args:
> +            existing = os.environ.get(b'EXTRA_TEST_ARGS', '')
> +            env[b'EXTRA_TEST_ARGS'] = existing + ' '.join(extra_args)

If existing is 'foo' and extra_args is ['bar'], don't we get 'foobar' instead of 'foo bar' here?

Comment 5

[Gregory Szorc [:gps]]

Good catch. Is that the only thing standing between this patch and r+?

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 688603 [details] [diff] [review]
Add --debugger option to run reftests, v1

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

Yeah, looks good otherwise.

Comment 7

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/7e5fee8e5ba0