"mach xpcshell-test" is difficult to use for TSan builds, for two reasons: (1) a TSan-ified process returns with a non-zero exit code if any races are detected. This causes the framework to incorrectly conclude that some processes have failed. (2) mach xpcshell-test ignores any existing setting of the TSAN_OPTIONS environment variable, and sets its own value for it. This makes xpcshell-tests difficult to use in practice for TSan runs, because it is often useful to be able to specify, for example, a custom suppression file using TSAN_OPTIONS.

Comment on attachment 8683769 [details] [diff] [review] WIP patch. Addresses both issues in comment #0. Review of attachment 8683769 [details] [diff] [review]: ----------------------------------------------------------------- Seems sensible to me. Move the 66 to a global and explain it there. Put the operators in binary expressions that split over multiple lines before the linebreak rather than right after.

Comment on attachment 8683769 [details] [diff] [review] WIP patch. Addresses both issues in comment #0. Review of attachment 8683769 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! Please fix my nit and also address jgraham's comment (move the 66 into a global TSAN_RETURN_CODE variable or similar along with the explanation). ::: testing/xpcshell/runxpcshelltests.py @@ +936,5 @@ > + and self.env["TSAN_OPTIONS"] is not None): > + tsanExtraOpts = " " + self.env["TSAN_OPTIONS"] > + > + self.env["TSAN_OPTIONS"] = ("external_symbolizer_path=%s" % llvmsym > + + tsanExtraOpts) Just make this whole block (I assume the potential extra space at the end is ok): self.env['TSAN_OPTIONS'] = "external_symbolizer_path={} {}".format(llvmsym, self.env.get('TSAN_OPTIONS', ''))