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: only test changed packages#1194

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
appgurueu merged 10 commits intoTheAlgorithms:masterfromdefaude:master
Oct 20, 2022
Merged

tests: only test changed packages#1194

appgurueu merged 10 commits intoTheAlgorithms:masterfromdefaude:master
Oct 20, 2022

Conversation

defaude
Copy link
Contributor

Relates to#1193

Copy link
Collaborator

@appgurueuappgurueu left a comment

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
Copy link
ContributorAuthor

However, having this long-running tests clogs up everyone's workflow and not "just" the CI, therefore I recommend tonot 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 changesOct 15, 2022
@appgurueu
Copy link
Collaborator

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

@defaude
Copy link
ContributorAuthor

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

appgurueu reacted with thumbs up emoji

@raklaptudirm
Copy link
Member

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

@defaude
Copy link
ContributorAuthor

Jest has anonlyChanged 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
Member

That could be great!

@defaude
Copy link
ContributorAuthor

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

appgurueu
appgurueu previously approved these changesOct 17, 2022
Copy link
Collaborator

@appgurueuappgurueu left a comment

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!

@raklaptudirmraklaptudirm added dependenciesPull requests that dependencies testsAdds or fixes tests; issue that points out bugs in the tests choreGeneral improvement labelsOct 17, 2022
@raklaptudirmraklaptudirm changed the titleSkip test that's running suuuuuuuper-longtests: skip super long oneOct 17, 2022
@raklaptudirmraklaptudirm changed the titletests: skip super long onetests: only test changed packagesOct 17, 2022
@raklaptudirm
Copy link
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
Copy link
ContributorAuthor

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

@defaude
Copy link
ContributorAuthor

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

@raklaptudirm
Copy link
Member

I guess I could set that up with GitHub Actions, yeah. But as long as#1193 is not solved, themaster 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.

@appgurueuappgurueu merged commit16fa774 intoTheAlgorithms:masterOct 20, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@appgurueuappgurueuappgurueu approved these changes

@raklaptudirmraklaptudirmraklaptudirm approved these changes

Assignees
No one assigned
Labels
choreGeneral improvementdependenciesPull requests that dependenciestestsAdds or fixes tests; issue that points out bugs in the tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@defaude@appgurueu@raklaptudirm

[8]ページ先頭

©2009-2025 Movatter.jp