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

[1.x] Run Zinc tests against all combination of major OS & Java version #1440

Closed
wants to merge 2 commits into from

Conversation

Friendseeker
Copy link
Member

@Friendseeker Friendseeker commented Oct 7, 2024

This PR modifies Github Action config to run Zinc test for combination of all LTS Java version on 3 major OS (Windows, Mac OS, Linux).

This helps with catching two types of issues.

Concurrency Issues

Concurrency issues are hard to detect as they may only manifest occasionally under specific condition. If we do not catch new concurrency issue in Pull Request CI, it becomes significantly harder to fix down the road. One such example is #1456.

By running Zinc test against all major configurations, we are more likely to catch concurrency issues.

OS / JVM specific issues

Different OS / JVM has subtle behavioural differences, which, if handled improperly, can cause bugs such as #1451. Testing against all major configurations mitigates such issues.

@Friendseeker
Copy link
Member Author

4 CI jobs failed at the same place... At least on CI it is readily reproducible.

[info] *** 1 TEST FAILED ***
[error] Failed: Total 40, Failed 1, Errors 0, Passed 39
[error] Failed tests:
[error] 	sbt.internal.inc.javac.JavaCompilerSpec

https://github.com/sbt/zinc/actions/runs/11207741451/job/31150376919?pr=1440

Definitely an actual bug with Zinc. Shall start an investigation....

@Friendseeker
Copy link
Member Author

Friendseeker commented Oct 7, 2024

Shall just conditionally disable the test for Java 17 & 21 windows machine and log a ticket for now.

@adpi2
Copy link
Member

adpi2 commented Oct 21, 2024

This creates 24 jobs which is more than the number of concurrent jobs we can have with a free Github plan: https://docs.github.com/en/actions/administering-github-actions/usage-limits-billing-and-administration#usage-limits

Do we really need to run all these jobs?

@Friendseeker
Copy link
Member Author

Friendseeker commented Oct 21, 2024

This creates 24 jobs which is more than the number of concurrent jobs we can have with a free Github plan: https://docs.github.com/en/actions/administering-github-actions/usage-limits-billing-and-administration#usage-limits

Do we really need to run all these jobs?

Ehh I didn't know there's job limit tied to an account. I somehow just assumed they are tied to popularity of a GitHub repo. And since I use a student account I guess I was granted higher limit and hence never noticed the issue.

I still feel that we need to add more coverage though (like this PR caught previously unnoticed JavaDoc output parsing issue). But I guess we need to add coverage in a more strategic way.

@mzuehlke
Copy link

Solution could be the merging of (some of) the jobtypes.

@Friendseeker
Copy link
Member Author

Solution could be the merging of (some of) the jobtypes.

Thanks! Shall try merging Job Type 1 & 2.

@Friendseeker Friendseeker marked this pull request as draft October 22, 2024 06:15
@Friendseeker
Copy link
Member Author

Even after merging job types there are still 16 jobs spawned...

Shall close the PR for now and rethink how to improve coverage without adding more runners. Initially I thought more runners have no drawback, but I guess parallelism always demonstrates Murphy's law. One does not simply add more test runners running in parallel....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants