Closed Bug 364043 Opened 13 years ago Closed 13 years ago

Remove dependency on Python for HTTP server to serve mochitests

Categories

(Testing :: Mochitest, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9

People

(Reporter: Waldo, Assigned: Waldo)

References

Details

Attachments

(1 file, 14 obsolete files)

80.37 KB, patch
rcampbell
: review+
Details | Diff | Splinter Review
Patch coming to use the new HTTP server that's used in netwerk unit tests...
Attached patch Patch (obsolete) — Splinter Review
This actually works 95% of the way.  There are the few problems I know about:

1. Uses wget to determine when the HTTP server is up -- don't know what Perl
   equivalent would be preferred, if any.  Alternately, we could let them race
   and maybe get lucky; the timeout's not a problem with debug builds, but I
   don't know whether it is in opt builds.  Racing seems like a bad idea.
2. HTTP server not shut down when you manually exit the browser during or after
   test execution.  This problem doesn't exist if you add --close-when-done to
   the runtests.pl arguments.  My Perl-fu is not great, but I don't see why this
   doesn't work.  (In contrast if you insert a sleep 100 in the server startup,
   the timeout correctly kills everything.)  Anyone else more familiar with Perl
   fork and pids?
3. The commandline interface got worse.  Perhaps too much.
4. One would think there's a way around the native-topsrcdir hack, but I'm not
   aware of it.
5. The example test from bug 359754 fails on about 40% of test runs.  Re-running
   usually makes it pass.  I don't know why -- I suspect it might be a problem
   with the test making some unreasonable assumption, but I don't know what it
   could be.  Alternately, it might just be trunk brokenness.
6. My Perl's probably not idiomatic (but hey, it's strict with warnings!).
Attachment #248824 - Flags: review?(sayrer)
Status: NEW → ASSIGNED
Comment on attachment 248824 [details] [diff] [review]
Patch

>-     It runs at http://localhost:8888/. Control-C will kill it. We are looking
>-     at other server options, because we don't want to require python.
>+     $ perl runtests.pl \
>+         '--xpcshell=<path-to-xpcshell>' 
>+         --topsrcdir=<path-to-topsrcdir>
>+         --native-topsrcdir=<path-to-topsrcdir-native-format>

...

>+
>+     We're aware that this step sucks (a lot).  Changes to perhaps get a build
>+     target for this are in the works.

I like this patch in general, but it's not going anywhere until we remove all of those switches. I realize this means we need build hacking to get those paths. However, Perl should be able transform the path formats as needed.
Attachment #248824 - Flags: review?(sayrer) → review-
We need to fix bug 359999 first.
Depends on: 359999
Blocks: 366712
Attached patch moving along... (obsolete) — Splinter Review
this is Jeff's code hacked to work with the patch in bug 359999.

I removed the need for the topsrcdir stuff by populating it with some Makefile junk, and removed the server's dependency on it by having it determine its path from CurProcD.
Attachment #248824 - Attachment is obsolete: true
the patch also puts the test profile right next runtests.pl. It probably doesn't work right now, but I've gotten this FindBin stuff to work on Windows before. Will need to check it.
Attached patch server running on windows (obsolete) — Splinter Review
this works on windows. I removed the method that pings the server, and instead had server.js write a file to the profile after it starts up. That loses the wget/LWP dependency.
Attachment #251268 - Attachment is obsolete: true
Comment on attachment 251979 [details] [diff] [review]
server running on windows

Step 2 needs updating in the README.


>Index: runtests.pl.in

>+  # builds.  We'll try to connect to the server for 30 seconds or until we

This actually should be 15s -- typo in my original patch.


>+  # add userChrome.css
>+  open(CRHOMEOUTFILE, ">>$chrome_dir/userChrome.css") || die("Could not open userChrome.css file $!");
>+  print CRHOMEOUTFILE ($chrome_content);
>+  close(CRHOMEOUTFILE);

s/RHOME/HROME/ here


>+  eval {
>+      my $loop_count = 0;
>+      while ($loop_count++ < $timeout) {
>+          last if (-e "$profile_dir/server_alive.txt");
>+          sleep 1;
>+      }
>+
>+      die "timeout" if ($loop_count >= $timeout);
>+      return "done";
>+  };

There's still a race here between the file being created and the server entering the event loop that actually causes it to respond to requests.  Maybe I should add a listener to the start() method which is called when the server is up to make this easier.  (I never intended that the external interface of the server really expose any asynchronicity, for conceptual simplicity, but my opinion is slowly moving in the opposite direction.  :-\ )


>Index: tests/index.html

>       'test_bug364413.xhtml',
>+<<<<<<< index.html
>+      'test_bug365773.xul',
>+=======
>       'test_bug365773.xul',
>       'test_bug366645.xhtml',
>       'test_bug367164.html,
>+>>>>>>> 1.55
>       'test_MochiKit-Async.html',

CVS conflict ;-)
Attached patch running on mac and windows (obsolete) — Splinter Review
Attachment #251979 - Attachment is obsolete: true
Depends on: 367537
Target Milestone: --- → mozilla1.9alpha
(In reply to comment #1)
> Created an attachment (id=248824) [details]
> 2. HTTP server not shut down when you manually exit the browser during or after
>    test execution.  ... Anyone else more familiar with
> Perl fork and pids?

OK. We seem to be getting the wrong pid here. Investigating.
Seems to be the pid of the forked runtests.pl process instead of the xpcshell process.
Attached patch kill child server process (obsolete) — Splinter Review
needed to use exec instead of system inside a fork.

Killing the run-mozilla.sh shell script doesn't kill the xpcshell process, so we'll need to set some env variables where that's necessary (linux, I think).
Attachment #252089 - Attachment is obsolete: true
Depends on: 367608
Attachment #252121 - Attachment is obsolete: true
I found myself wanting a templating language. Ack. 
Instead, I did HTML generation as follows.

This function:

function defaultDirHandler(metadata, response)
{
  response.setStatusLine("1.1", 200, "OK");
  response.setHeader("Content-type", "text/html");

  response.write(
    HTML(
      HEAD(
        TITLE("mochitest index ", metadata.path)
      ),
      BODY(
        UL(
          LI("foo"),
          LI(
            P("bar"),
            IMG({src:"..."})
          ),
          LI("baz")
        )
      )
    )
  );
}

creates this output:

<HTML>
 <HEAD>
  <TITLE>mochitest index</TITLE>
 </HEAD>
 <BODY>
  <UL>
   <LI>foo</LI>
   <LI>
     <P>bar</P>
     <IMG src="..."></IMG>
   </LI>
   <LI>baz</LI>
  </UL>
 </BODY>
</HTML>
Attachment #252179 - Attachment is obsolete: true
Attached patch Operational Test Harness (obsolete) — Splinter Review
This now copies the MochiKit unit tests into the objdir and executes the tests as before.
Attachment #252244 - Attachment is obsolete: true
Attached patch WIP - faster harness (obsolete) — Splinter Review
Attachment #252272 - Attachment is obsolete: true
Attachment #252299 - Attachment is obsolete: true
Attached patch Nits. (obsolete) — Splinter Review
Attachment #252354 - Attachment is obsolete: true
Attachment #252356 - Flags: review?(rcampbell)
This requires the new patch in bug 367537, and eliminates the dependency on bug 367608.
Attachment #252356 - Attachment is obsolete: true
Attachment #252499 - Flags: review?(rcampbell)
Attachment #252356 - Flags: review?(rcampbell)
Attachment #252499 - Attachment is obsolete: true
Attachment #252500 - Flags: review?(rcampbell)
Attachment #252499 - Flags: review?(rcampbell)
No longer depends on: 367608
Attachment #252500 - Attachment is obsolete: true
Attachment #252671 - Flags: review?(rcampbell)
Attachment #252500 - Flags: review?(rcampbell)
Attached patch --appname worksSplinter Review
Attachment #252671 - Attachment is obsolete: true
Attachment #252701 - Flags: review?(rcampbell)
Attachment #252671 - Flags: review?(rcampbell)
Attachment #252701 - Flags: review?(rcampbell) → review+
Checking in Makefile.in;
/cvsroot/mozilla/testing/mochitest/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done
Checking in README.txt;
/cvsroot/mozilla/testing/mochitest/README.txt,v  <--  README.txt
new revision: 1.6; previous revision: 1.5
done
Removing runtests.pl;
/cvsroot/mozilla/testing/mochitest/runtests.pl,v  <--  runtests.pl
new revision: delete; previous revision: 1.10
done
RCS file: /cvsroot/mozilla/testing/mochitest/server.js,v
done
Checking in server.js;
/cvsroot/mozilla/testing/mochitest/server.js,v  <--  server.js
initial revision: 1.1
done
Checking in static/Makefile.in;
/cvsroot/mozilla/testing/mochitest/static/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/mochitest/static/harness.css,v
done
Checking in static/harness.css;
/cvsroot/mozilla/testing/mochitest/static/harness.css,v  <--  harness.css
initial revision: 1.1
done
Checking in tests/Makefile.in;
/cvsroot/mozilla/testing/mochitest/tests/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Async.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Async.html,v  <--  test_MochiKit-Async.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Base.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Base.html,v  <--  test_MochiKit-Base.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Color.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Color.html,v  <--  test_MochiKit-Color.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-DOM.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-DOM.html,v  <--  test_MochiKit-DOM.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-DateTime.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-DateTime.html,v  <--  test_MochiKit-DateTime.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-DragAndDrop.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-DragAndDrop.html,v  <--  test_MochiKit-DragAndDrop.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Format.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Format.html,v  <--  test_MochiKit-Format.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Iter.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Iter.html,v  <--  test_MochiKit-Iter.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Logging.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Logging.html,v  <--  test_MochiKit-Logging.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-MochiKit.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-MochiKit.html,v  <--  test_MochiKit-MochiKit.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Signal.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Signal.html,v  <--  test_MochiKit-Signal.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Style.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Style.html,v  <--  test_MochiKit-Style.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/test_MochiKit-Visual.html;
/cvsroot/mozilla/testing/mochitest/tests/test_MochiKit-Visual.html,v  <--  test_MochiKit-Visual.html
new revision: 1.2; previous revision: 1.1
done
Checking in tests/SimpleTest/Makefile.in;
/cvsroot/mozilla/testing/mochitest/tests/SimpleTest/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done
Checking in tests/SimpleTest/MozillaFileLogger.js;
/cvsroot/mozilla/testing/mochitest/tests/SimpleTest/MozillaFileLogger.js,v  <--  MozillaFileLogger.js
new revision: 1.2; previous revision: 1.1
done
Checking in tests/SimpleTest/TestRunner.js;
/cvsroot/mozilla/testing/mochitest/tests/SimpleTest/TestRunner.js,v  <--  TestRunner.js
new revision: 1.3; previous revision: 1.2
done
Checking in tests/SimpleTest/quit.js;
/cvsroot/mozilla/testing/mochitest/tests/SimpleTest/quit.js,v  <--  quit.js
new revision: 1.2; previous revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/mochitest/tests/SimpleTest/setup.js,v
done
Checking in tests/SimpleTest/setup.js;
/cvsroot/mozilla/testing/mochitest/tests/SimpleTest/setup.js,v  <--  setup.js
initial revision: 1.1
done
Checking in runtests.pl.in;
/cvsroot/mozilla/testing/mochitest/runtests.pl.in,v  <--  runtests.pl.in
initial revision: 1.1
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Component: Testing → Mochitest
Product: Core → Testing
QA Contact: testing → mochitest
Target Milestone: mozilla1.9alpha1 → mozilla1.9
Depends on: 473506
You need to log in before you can comment on or make changes to this bug.