Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Tests: Fix flakiness in the "jQuery.ajax() - JSONP - Same Domain" test#4687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mgol merged 1 commit intojquery:masterfrommgol:jsonp-test-flakiness
Apr 27, 2020

Conversation

mgol
Copy link
Member

Summary

The "jQuery.ajax() - JSONP - Same Domain" test is firing a request with
a duplicate "callback" parameter, something like (simplified):

mock.php?action=jsonp&callback=jQuery_1&callback=jQuery_2

There was a difference in how the PHP & Node.js implementations of the jsonp
action in the mock server handled situations like that. The PHP implementation
was using the latest parameter while the Node.js one was turning it into an
array but the code didn't handle this situation. Because of how JavaScript
stringifies arrays, while the PHP implementation injected the following code:

jQuery_2(payload)

the Node.js one was injecting the following one:

jQuery_1,jQuery_2(payload)

This is a comma expression in JavaScript; it so turned out that in the majority
of cases both callbacks were identical so it was more like:

jQuery_1,jQuery_1(payload)

which evaluates tojQuery_1(payload) whenjQuery_1 is defined, making the
test go as expected. In many cases, though, especially on Travis, the callbacks
were different, triggering anUncaught ReferenceError error & requiring
frequent manual re-runs of Travis builds.

This commit fixes the logic in the mock Node.js server, adding special handling
for arrays.

Checklist

Krinkle reacted with thumbs up emoji
The "jQuery.ajax() - JSONP - Same Domain" test is firing a request witha duplicate "callback" parameter, something like (simplified):```mock.php?action=jsonp&callback=jQuery_1&callback=jQuery_2```There was a difference in how the PHP & Node.js implementations of the jsonpaction in the mock server handled situations like that. The PHP implementationwas using the latest parameter while the Node.js one was turning it into anarray but the code didn't handle this situation. Because of how JavaScriptstringifies arrays, while the PHP implementation injected the following code:```jsjQuery_2(payload)```the Node.js one was injecting the following one:```jsjQuery_1,jQuery_2(payload)```This is a comma expression in JavaScript; it so turned out that in the majorityof cases both callbacks were identical so it was more like:```jsjQuery_1,jQuery_1(payload)```which evaluates to `jQuery_1(payload)` when `jQuery_1` is defined, making thetest go as expected. In many cases, though, especially on Travis, the callbackswere different, triggering an `Uncaught ReferenceError` error & requiringfrequent manual re-runs of Travis builds.This commit fixes the logic in the mock Node.js server, adding special handlingfor arrays.
@mgolmgol added this to the3.5.1 milestoneApr 24, 2020
@mgolmgol self-assigned thisApr 24, 2020
@mgol
Copy link
MemberAuthor

An example failing build (with some debug logging details):https://travis-ci.org/github/jquery/jquery/jobs/678811015

You can see the error in the logs:

HeadlessChrome 81.0.4044 (Linux 0.0.0) ERROR  {    "message": "Uncaught ReferenceError: jQuery008281692735376822_1587716819366 is not defined\nat test/data/mock.php=jsonp&callback=jQuery008281692735376822_1587716819366&&callback=jQuery008281692735376822_1587716819395&_=1587716819423:1:1\n\nReferenceError: jQuery008281692735376822_1587716819366 is not defined\n    at test/data/mock.php=jsonp&callback=jQuery008281692735376822_1587716819366&&callback=jQuery008281692735376822_1587716819395&_=1587716819423:1:1",    "str": "Uncaught ReferenceError: jQuery008281692735376822_1587716819366 is not defined\nat test/data/mock.php=jsonp&callback=jQuery008281692735376822_1587716819366&&callback=jQuery008281692735376822_1587716819395&_=1587716819423:1:1\n\nReferenceError: jQuery008281692735376822_1587716819366 is not defined\n    at test/data/mock.php=jsonp&callback=jQuery008281692735376822_1587716819366&&callback=jQuery008281692735376822_1587716819395&_=1587716819423:1:1"  }HeadlessChrome 81.0.4044 (Linux 0.0.0): Executed 771 of 1079 (skipped 1) ERROR (33.753 secs / 23.364 secs)

@mgol
Copy link
MemberAuthor

It took me a few months of fighting with this issue to finally take the time to find the culprit. 😅

Krinkle reacted with thumbs up emoji

Copy link
Member

@dmethvindmethvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Your persistence paid off!

@mgolmgol merged commit7b0864d intojquery:masterApr 27, 2020
@mgolmgol deleted the jsonp-test-flakiness branchApril 27, 2020 18:22
mgol added a commit that referenced this pull requestApr 27, 2020
The "jQuery.ajax() - JSONP - Same Domain" test is firing a request witha duplicate "callback" parameter, something like (simplified):```mock.php?action=jsonp&callback=jQuery_1&callback=jQuery_2```There was a difference in how the PHP & Node.js implementations of the jsonpaction in the mock server handled situations like that. The PHP implementationwas using the latest parameter while the Node.js one was turning it into anarray but the code didn't handle this situation. Because of how JavaScriptstringifies arrays, while the PHP implementation injected the following code:```jsjQuery_2(payload)```the Node.js one was injecting the following one:```jsjQuery_1,jQuery_2(payload)```This is a comma expression in JavaScript; it so turned out that in the majorityof cases both callbacks were identical so it was more like:```jsjQuery_1,jQuery_1(payload)```which evaluates to `jQuery_1(payload)` when `jQuery_1` is defined, making thetest go as expected. In many cases, though, especially on Travis, the callbackswere different, triggering an `Uncaught ReferenceError` error & requiringfrequent manual re-runs of Travis builds.This commit fixes the logic in the mock Node.js server, adding special handlingfor arrays.Closesgh-4687(cherry picked from commit7b0864d)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dmethvindmethvindmethvin approved these changes

Assignees

@mgolmgol

Labels
Milestone
3.5.1
Development

Successfully merging this pull request may close these issues.

2 participants
@mgol@dmethvin

[8]ページ先頭

©2009-2025 Movatter.jp