Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: Exclude tests based on compilation flags, not API presence #5069

Closed
mgol opened this issue Jun 28, 2022 · 4 comments · Fixed by #5046
Closed

Tests: Exclude tests based on compilation flags, not API presence #5069

mgol opened this issue Jun 28, 2022 · 4 comments · Fixed by #5046
Assignees
Milestone

Comments

@mgol
Copy link
Member

mgol commented Jun 28, 2022

Description

We shouldn't treat presence of particular APIs to decide whether to run a test as then if we accidentally remove an API, the tests would still not fail. This has already happened when we removed String.prototype.trim on master without its tests and nothing failed since the tests were being guarded by a check for the existence of this method.

Tests should detect which modules are included in the current compilation by inspecting jQuery.fn.jquery and decide which tests to run based on that.

Link to test case

@mgol mgol added this to the 3.6.1 milestone Jun 28, 2022
@mgol mgol self-assigned this Jun 28, 2022
@mgol
Copy link
Member Author

mgol commented Jun 28, 2022

PR for main: #5046

mgol added a commit to mgol/jquery that referenced this issue Jun 28, 2022
Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Fixes jquerygh-5069
Closes jquerygh-5046
mgol added a commit to mgol/jquery that referenced this issue Jun 28, 2022
Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Fixes jquerygh-5069
Closes jquerygh-5046
mgol added a commit to mgol/jquery that referenced this issue Jun 28, 2022
Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Fixes jquerygh-5069
Closes jquerygh-5046
@mgol mgol closed this as completed in fae5fee Jun 28, 2022
@mgol
Copy link
Member Author

mgol commented Jun 28, 2022

Reopening until the 3.x-stable version lands.

@mgol mgol reopened this Jun 28, 2022
mgol added a commit to mgol/jquery that referenced this issue Jun 28, 2022
Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Fixes jquerygh-5069
Ref jquerygh-5046

(partially cherry picked from commit fae5fee)
mgol added a commit to mgol/jquery that referenced this issue Jun 28, 2022
Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Fixes jquerygh-5069
Ref jquerygh-5046

(partially cherry picked from commit fae5fee)
@mgol
Copy link
Member Author

mgol commented Jun 28, 2022

3.x PR: #5071.

mgol added a commit to mgol/jquery that referenced this issue Jun 28, 2022
Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Fixes jquerygh-5069
Ref jquerygh-5046

(partially cherry picked from commit fae5fee)
mgol added a commit that referenced this issue Jul 12, 2022
…x version)

Introduces a new test API, `includesModule`. The method returns whether
a particular module like "ajax" or "deprecated" is included in the current
jQuery build; it handles the slim build as well. The util was created so that
we don't treat presence of particular APIs to decide whether to run a test as
then if we accidentally remove an API, the tests would still not fail.

Closes gh-5071
Fixes gh-5069
Ref gh-5046

(partially cherry picked from commit fae5fee)
@mgol
Copy link
Member Author

mgol commented Jul 12, 2022

All PRs have landed; closing.

@mgol mgol closed this as completed Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants
@mgol and others