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

ci: Add ability to exclude tests via ID in testExclusionList.json #8774

Merged
merged 9 commits into from
Oct 18, 2023

Conversation

ddrechse
Copy link
Contributor

@ddrechse ddrechse commented Oct 11, 2023

Pull Request

Issue

Add support for testExclusionList so different adapter repos can exclude tests specific to that adapter.
Move test exclusion out of the main parse-server repo
Closes: #8714

Approach

This PR adds the ability in helper.js to exclude tests based on a uuid
A file, testExclusionList is created in the spec dir and looks like

[
"bbd9e2f6-7f61-458f-98f2-4a563586cd8d",
"e1e86b38-b8a4-4109-8330-a324fe628e0c",
"05f1a454-56b1-4f2e-908e-408a9222cbae",
"9ee376ad-dd6c-4c17-ad28-c7899a4411f1"
]

and tests to be excluded are prefixed like
it_id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d')('geo line', async done => {

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 11, 2023

Thanks for opening this pull request!

spec/helper.js Show resolved Hide resolved
@mtrezza mtrezza changed the title Add testExclusionList support so tests can be excluded using a uuid ci: Add ability to exclude tests via ID in testExclusionList.json Oct 11, 2023
spec/helper.js Outdated Show resolved Hide resolved
spec/helper.js Outdated Show resolved Hide resolved
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
spec/helper.js Outdated Show resolved Hide resolved
spec/helper.js Outdated Show resolved Hide resolved
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
mtrezza
mtrezza previously approved these changes Oct 12, 2023
@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2023

Looks good, could you try again with the refactor I've done, to make sure it works?

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7d32d89) 94.33% compared to head (170a5d3) 93.95%.
Report is 1 commits behind head on alpha.

❗ Current head 170a5d3 differs from pull request most recent head 9067f1d. Consider uploading reports for the commit 9067f1d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #8774      +/-   ##
==========================================
- Coverage   94.33%   93.95%   -0.38%     
==========================================
  Files         186      186              
  Lines       14780    14780              
==========================================
- Hits        13942    13886      -56     
- Misses        838      894      +56     

see 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2023

Noted that this implementation won't work for tests that use any of the custom its, like it_only_node_version.

To address this, we could:

  • a) Add it_id_only_node_version, which takes as arguments the id and the node version. However, that seems like duplicating code unnecessarily, requiring a separate method with ID for every other overloaded it method.
  • b) Rewrite the it_id to not call it if the ID is not in the exclusion list but just execute whatever function is passed to it as an argument. The syntax would change from it_id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d')('example test', async () => {}); to id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d', it_only_node_version('example test', async () => {}));, and could be simplified to id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d', it_only_node_version, 'example test', async () => {}); for less brackets. If nothing should be executed we could still execute a xid with just the ID as name, so that it's logged in the CI that the test with that ID has been skipped.

@ddrechse
Copy link
Contributor Author

added the suggested changes, tested, lgtm

@mtrezza
Copy link
Member

mtrezza commented Oct 14, 2023

Did you consider #8774 (comment)?

@ddrechse
Copy link
Contributor Author

I added the requested support
it_id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d')('geo line', async done => {
Still works

it_id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d',it_only_node_version("v16.20.1"))('geo line', async done => {
works also
This ends up calling it_only_node_version first and that return value it/xit is passed into it_id

The test exclusion list overrides all
I tested and lgtm

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! Waiting for CI to pass...

@mtrezza mtrezza merged commit 93af48a into parse-community:alpha Oct 18, 2023
23 of 24 checks passed
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.4.0-alpha.6

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 18, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.5.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 16, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.5.0-alpha.1

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.5.0

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test exclusion list
3 participants