Closed Bug 424813 Opened 12 years ago Closed 12 years ago

Update Mochitest jQuery Test Suite to 1.2.3

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jeresig, Assigned: harth)

References

Details

Attachments

(1 file, 3 obsolete files)

The test suite that we have for jQuery is out of date, at this point, it should be upgraded to the latest version: 1.2.3.
Updated Mochitest JQuery test suite to version 1.2.6. One test fails in the current tree:
  105. ajax module: $.ajax() - error callbacks (2, 6, 8) 
   4. success
   5. ajaxSuccess

Also, there seems to be a couple minutes of waiting before the mochitest harness registers the successes and failures. There may be some redundant code/directories as well(e.g. the src directory).
Assignee: nobody → harthur
Status: NEW → ASSIGNED
Attachment #324334 - Flags: review?(jresig)
So part of the jQuery test suite uses server-side PHP files in order to generate appropriate responses - I think that may be the case here (and I'm fairly certain that we aren't capable of running PHP in Mochitest). How many Ajax tests are running? Is this the only one? If there's a bunch running, and passing, then that would mean that the PHP is working as you would expect it to.
Blocks: 442369
Attached patch patch for JQuery test suite (obsolete) — Splinter Review
I commented out the tests that required php scripts. However, the error callback failure seems to be unrelated to this issue. I separated this failure out into bug 442369. All tests pass in this patch.
Attachment #324334 - Attachment is obsolete: true
Attachment #327176 - Flags: review?(jresig)
Attachment #324334 - Flags: review?(jresig)
(In reply to comment #3)
Could you also attach the binary files needed by this build? I just tried a build and it failed:
make[10]: *** No rule to make target `cow.jpg', needed by `libs'.  Stop.
make[9]: *** [libs] Error 2
make[8]: *** [libs] Error 2
make[7]: *** [libs] Error 2
make[6]: *** [libs] Error 2
make[5]: *** [libs] Error 2
make[4]: *** [libs] Error 2
make[3]: *** [libs_tier_gecko] Error 2
make[2]: *** [tier_gecko] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2

You'll need to attach them individually (or in an archive) - please specify the file location for them, as well.
(In reply to comment #4)
> You'll need to attach them individually (or in an archive) - please specify the
> file location for them, as well.
The file is actually in the patch.  If you import the patch file into your patch queue it should work.
I guess that means I need to switch to Mercurial. Oooo K. I'll get back to you.
I just tried hg patch -p1 bug424813.diff on a clean pull of mozilla-central and it worked. Sorry, I did not specify that it was mercurial )-:
It's ok, I'm trying to get a good Mercurial set up going now - after which I'll go back and give bug 424814 a try, again.
Comment on attachment 327176 [details] [diff] [review]
patch for JQuery test suite

In dom/tests/mochitest/ajax/jquery/test/data/Makefile.in you have some weird tab and spaces issues.  It looks like it should be tabs based on what used to be there.

Should the php files be included since we clearly aren't running them?

Once I run these locally, I'll do the review.  Please address the above comments though.

jresig - any problem with me doing the review of this instead of you so we can get this landed?
Attachment #327176 - Flags: review?(sdwilsh)
Updated patch without php files and redundant dist files. 

remove these empty directories after applying this patch:
dom/tests/mochitest/ajax/jquery/src/ajax
dom/tests/mochitest/ajax/jquery/src/event
dom/tests/mochitest/ajax/jquery/src/fx
dom/tests/mochitest/ajax/jquery/src/jquery
dom/tests/mochitest/ajax/jquery/src/selector
Attachment #327176 - Attachment is obsolete: true
Attachment #332501 - Flags: review?(sdwilsh)
Attachment #327176 - Flags: review?(sdwilsh)
Attachment #327176 - Flags: review?(jresig)
Comment on attachment 332501 [details] [diff] [review]
patch for JQuery test suite minus php files

I should have caught this last time too.  It looks like the speed stuff does not actually do any testing, just performance numbers which doesn't make much sense for us to have.
Comment on attachment 332501 [details] [diff] [review]
patch for JQuery test suite minus php files

r=sdwilsh with the performance tests removed.
Attachment #332501 - Flags: review?(sdwilsh) → review+
patch without speed directory and redundant src directory (tests use dist/jquery.js). All tests pass.
Attachment #332501 - Attachment is obsolete: true
I'm assuming that jresig doesn't have a problem with my review being sufficient.
Keywords: checkin-needed
(In reply to comment #14)
> I'm assuming that jresig doesn't have a problem with my review being
> sufficient.
> 
Shawn, thanks for the r+, do you want to check this in, or do you want me to do it?
If you can do it now, please do.  I'm not going to be able to do this for a few hours still.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
Depends on: 450009
Component: Testing → DOM
QA Contact: testing → general
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.