Closed Bug 539516 Opened 10 years ago Closed 10 years ago

Switch automation.py to use the new fix-macosx-stack.py (rather than .pl)

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set

Tracking

(status1.9.2 .4-fixed)

RESOLVED FIXED
Tracking Status
status1.9.2 --- .4-fixed

People

(Reporter: jruderman, Assigned: jruderman)

References

Details

(Whiteboard: [fixed-lorentz])

Attachments

(1 file, 4 obsolete files)

Attached patch first attempt (obsolete) — Splinter Review
I used this last night and it sort of worked.  But the makefile changes are probably wrong.  And I'd like to make fix-macosx-stack.py work as a module so atos processes can be cached between browser invocations.
Blocks: 539518
Using fix-macosx-stack.py as a module is a huge speed boost for users of automation.py that call runApp multiple times.  Keeping the atos processes around only costs about 70MB of RAM.

I had to use __import__ rather than the import statement because Python doesn't like hyphens in identifiers.  I guess I could rename fix-macosx-stack.pl instead.

When we land this, we'll have to watch carefully for latent timing bugs tickled by Firefox's stdout pipe filling up while automation.py waits for atos.  This was probably a problem with the old script too, but the wait is longer the first time now.  I imagine there will be more test bugs than Firefox bugs.
Attachment #421523 - Flags: review?(ted.mielczarek)
Attachment #421493 - Attachment is obsolete: true
Attached patch patch updated to trunk (obsolete) — Splinter Review
Merged against bug 530475.
Attachment #421523 - Attachment is obsolete: true
Attachment #421969 - Flags: review?(ted.mielczarek)
Attachment #421523 - Flags: review?(ted.mielczarek)
Attached patch patch updated to trunk again (obsolete) — Splinter Review
Merged against the other patch in bug 530475.
Attachment #421969 - Attachment is obsolete: true
Attachment #423688 - Flags: review?
Attachment #421969 - Flags: review?(ted.mielczarek)
Attachment #423688 - Flags: review? → review?(ted.mielczarek)
It'd be cool to port the Linux code to Python as well, and merge these two codepaths.
Comment on attachment 423688 [details] [diff] [review]
patch updated to trunk again

I'm not really wild about the use of __import__ here. Do you think you could just  rename fix-macosx-stack.py to something that's importable, add it to automation-build.mk, then import it directly?

diff --git a/testing/mochitest/Makefile.in b/testing/mochitest/Makefile.in
--- a/testing/mochitest/Makefile.in
+++ b/testing/mochitest/Makefile.in
@@ -102,16 +102,17 @@ ifeq ($(OS_ARCH),WINNT)
 TEST_HARNESS_BINS += \
   crashinject$(BIN_SUFFIX) \
   crashinjectdll$(DLL_SUFFIX) \
   $(NULL)
 endif
 
 ifeq ($(OS_ARCH),Darwin)
 TEST_HARNESS_BINS += fix-macosx-stack.pl
+TEST_HARNESS_BINS += fix-macosx-stack.py
 endif

While you're making this change, drop fix-macosx-stack.pl from this makefile.
Renames fix-macosx-stack.py to fix_macosx_stack.py, but doesn't do the automation-build.mk thing.
Attachment #423688 - Attachment is obsolete: true
Attachment #425579 - Flags: review?(ted.mielczarek)
Attachment #423688 - Flags: review?(ted.mielczarek)
Attachment #425579 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 425579 [details] [diff] [review]
patch addressing most of ted's comments

This is fine, but any reason not to put it in automation-build.mk? Then you could just import it directly instead of pulling it from utilityPath.
Depends on: 544891
Fixed: http://hg.mozilla.org/mozilla-central/rev/bf75623a6e2d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
This doesn't help on Tinderbox, at least for reftest.  Filed bug 547646.
Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.