Skip to content

tests: only test changed packages - #1194

Merged
appgurueu merged 10 commits into
TheAlgorithms:masterfrom
defaude:master
Oct 20, 2022
Merged

tests: only test changed packages#1194
appgurueu merged 10 commits into
TheAlgorithms:masterfrom
defaude:master

Conversation

@defaude

@defaude defaude commented Oct 15, 2022

Copy link
Copy Markdown
Contributor

Relates to #1193

@appgurueu appgurueu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is akin to outcommenting the test.

Project Euler solutions can be expected to take some time (should be < one minute though, ideally the algo should be optimized).

If you do not want to run this test, simply exclude the directory when running tests using Jest.

@defaude

defaude commented Oct 15, 2022

Copy link
Copy Markdown
Contributor Author

However, having this long-running tests clogs up everyone's workflow and not "just" the CI, therefore I recommend to not just uncommenting the test but rather explicitly skipping it. This way, it's still visible in the console. Finding a solution to the runtime behavior is step 2 - but meanwhile, everyone else can continue working without that delay.

What do you think?

appgurueu
appgurueu previously approved these changes Oct 15, 2022
@appgurueu

Copy link
Copy Markdown
Collaborator

We'll have to leave the issue open to find a proper fix eventually however.

@defaude

defaude commented Oct 15, 2022

Copy link
Copy Markdown
Contributor Author

Yup! 👍 I mereley mentioned the issue no. in the commit message but didn't close it.

@raklaptudirm

Copy link
Copy Markdown
Member

Ideally, we would want to run the tests in the changed directories only.

@defaude

defaude commented Oct 16, 2022

Copy link
Copy Markdown
Contributor Author

Jest has an onlyChanged option that I've set up in the pre-commit hook. If that's what you like, we could also switch to that behavior for the CI, as well.

@raklaptudirm

Copy link
Copy Markdown
Member

That could be great!

@defaude

defaude commented Oct 16, 2022

Copy link
Copy Markdown
Contributor Author

@raklaptudirm I've updated the CI config, as well 👍

Comment thread CONTRIBUTING.md Outdated
appgurueu
appgurueu previously approved these changes Oct 17, 2022

@appgurueu appgurueu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your contribution!

@raklaptudirm raklaptudirm added dependencies Pull requests that dependencies tests Adds or fixes tests; issue that points out bugs in the tests chore General improvement labels Oct 17, 2022
@raklaptudirm raklaptudirm changed the title Skip test that's running suuuuuuuper-long tests: skip super long one Oct 17, 2022
@raklaptudirm raklaptudirm changed the title tests: skip super long one tests: only test changed packages Oct 17, 2022
@raklaptudirm

Copy link
Copy Markdown
Member

The pr looks fine, but I had an idea. I think we would want to run all the tests in the master branch, but run tests on only the changed files in the prs. What do you all think?

@defaude

defaude commented Oct 17, 2022

Copy link
Copy Markdown
Contributor Author

I guess I could set that up with GitHub Actions, yeah. But as long as #1193 is not solved, the master build will take forever to complete.

@defaude

defaude commented Oct 17, 2022

Copy link
Copy Markdown
Contributor Author

I've updated the Workflow accordingly - please take a look 😀

@raklaptudirm

Copy link
Copy Markdown
Member

I guess I could set that up with GitHub Actions, yeah. But as long as #1193 is not solved, the master build will take forever to complete.

The master build should not be an issue as no one will need to wait for that to complete.

Comment thread .github/workflows/Ci.yml Outdated
@appgurueu
appgurueu merged commit 16fa774 into TheAlgorithms:master Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore General improvement dependencies Pull requests that dependencies tests Adds or fixes tests; issue that points out bugs in the tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants