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.

Sign up for GitHub

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

Jump to bottom

Added framework matrix to build #1362

Merged
merged 24 commits into from
Nov 12, 2024
Merged

Added framework matrix to build #1362

merged 24 commits into from
Nov 12, 2024

Conversation

josephmyers
Copy link
Collaborator

@josephmyers josephmyers commented Nov 8, 2024 •
edited by andrew-polk
Loading

This change is Reviewable

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 8, 2024 •
edited
Loading

LibPalaso Tests

    1 files   -    44      1 suites   - 44   5m 6s ⏱️ - 6m 49s
4 824 tests  -     6  4 593 ✅  -     6  231 💤 ±  0  0 ❌ ±0 
4 843 runs   - 9 165  4 599 ✅  - 8 782  244 💤  - 383  0 ❌ ±0 

Results for commit 98ab77c. ± Comparison against base commit de37405.

This pull request removes 6 tests. 
SIL.Tests.ExtractCopyright.ExtractCopyrightTests ‑ CreateCopyrightFileWorks
SIL.Tests.ExtractCopyright.ExtractCopyrightTests ‑ ParseChangelogContentForValuesWorks
SIL.Tests.ExtractCopyright.ExtractCopyrightTests ‑ ParseControlContentForValuesWorks
SIL.Tests.ExtractCopyright.ExtractCopyrightTests ‑ ReadSimpleCopyrightFileWorks
SIL.Tests.ExtractCopyright.ExtractCopyrightTests ‑ UpdateCopyrightFileWorks
SIL.Tests.ExtractCopyright.ExtractCopyrightTests ‑ WriteSimpleCopyrightFileWorks

♻️ This comment has been updated with latest results.

@josephmyers
binlog silliness
1cc16a1
@josephmyers
Remove binlog, config, and no-build
1e8786b
@josephmyers
Added IsTestProject to test projects
ea87f64 
This is necessary so that attempting to run tests with unsupported frameworks will fail. It also just seems like something that should have been done. There may be other benefits (or consequences) to setting this, as well.
@josephmyers josephmyers mentioned this pull request Nov 8, 2024
Closed
@josephmyers
Copy link
Collaborator Author

@hahn-kev @rmunn I haven't yet looked into how to combine the test results from the three frameworks. Did either of you deal with this for Chorus?

@josephmyers
Copy link
Collaborator Author

@hahn-kev also note the mildly terrifying build failure here:
https://github.com/sillsdev/libpalaso/actions/runs/11739761597/job/32704918933?pr=1362

Are we running into another concurrency issue with this design?

@josephmyers
Only building the specified framework on that framework
3a13fb1 
For some reason even dotnet build can be flaky.
@josephmyers
Reverting that
98ab77c 
-f is much too brittle for our solution. It will fail the build if any project is missing that target. Maybe this means we still have the concurrency problems, just lessened. I'll keep investigating.
@josephmyers
Copy link
Collaborator Author

@hahn-kev also note the mildly terrifying build failure here: https://github.com/sillsdev/libpalaso/actions/runs/11739761597/job/32704918933?pr=1362

Are we running into another concurrency issue with this design?

We are. The build itself is flaky, where this work should fix the test flakiness. The build flakiness is hopefully much rarer

@hahn-kev
Copy link
Contributor

that's a really weird build failure. I wonder if it'll happen again

@josephmyers
Combining test results from all frameworks
700a633 
Using the pattern established in liblcm
@josephmyers
Upgrading EnricoMi
0bce0c8 
Trying to fix an exception for its step
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 11, 2024 •
edited
Loading

Palaso Tests

     3 files       3 suites   14m 9s ⏱️
 4 824 tests  4 593 ✅ 231 💤 0 ❌
13 996 runs  13 369 ✅ 627 💤 0 ❌

Results for commit 8a943d3.

♻️ This comment has been updated with latest results.

@josephmyers josephmyers requested a review from hahn-kev November 11, 2024 07:27
@josephmyers josephmyers marked this pull request as ready for review November 11, 2024 07:28
hahn-kev
hahn-kev requested changes Nov 11, 2024
View reviewed changes
Copy link
Contributor

@hahn-kev hahn-kev 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! Missed the bin log upload name though

.github/workflows/build.yml Outdated Show resolved Hide resolved
@josephmyers josephmyers requested a review from hahn-kev November 11, 2024 09:33
@josephmyers josephmyers merged commit c06c991 into master Nov 12, 2024
5 of 7 checks passed
@josephmyers josephmyers deleted the separate-tests-2 branch November 12, 2024 04:41
@josephmyers
Copy link
Collaborator Author

Note that the Appveyor build didn't fail, it wasn't canceled due to my merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hahn-kev hahn-kev hahn-kev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@josephmyers @hahn-kev