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

Add failure count track to Mill's execution and logging #4602

Closed
wants to merge 10 commits into from

Conversation

zelosleone
Copy link

This change introduces a mechanism to track and display the number of failed tasks during Mill's execution:

Added failureCount atomic integer in Execution to track task failures Updated PrefixLogger to support displaying failure count in the prompt header Implemented logic to increment failure count when tasks fail Added comprehensive test cases in PromptLoggerTests to verify failure count behavior The new feature provides more visibility into task execution by showing the number of failed tasks in the build output.Please open all PRs as drafts and ensure that your fork of Mill has settings/actions / Allow all actions and reusable workflows enabled to run CI on your own fork of the Mill repo. Only once CI passes mark the PR as Ready for review and CI will run on the main Mill repo before we merge it.
Solves #4588

This change introduces a mechanism to track and display the number of failed tasks during Mill's execution:

Added failureCount atomic integer in Execution to track task failures
Updated PrefixLogger to support displaying failure count in the prompt header
Implemented logic to increment failure count when tasks fail
Added comprehensive test cases in PromptLoggerTests to verify failure count behavior
The new feature provides more visibility into task execution by showing the number of failed tasks in the build output.Please open all PRs as drafts and ensure that your fork of Mill has
settings/actions / Allow all actions and reusable workflows enabled to run CI on
your own fork of the Mill repo. Only once CI passes mark the PR as Ready for review
and CI will run on the main Mill repo before we merge it.
@zelosleone zelosleone changed the title Add failure count tracking to Mill's execution and logging (#1) Add failure count track to Mill's execution and logging Feb 20, 2025
@zelosleone
Copy link
Author

@lihaoyi ready for review.

@lihaoyi
Copy link
Member

lihaoyi commented Feb 21, 2025

This doesn't seem to work

Screenshot 2025-02-21 at 9 06 48 AM

Please add a test case to integration/feature/full-run-logs/src/FullRunLogsTests.scala that uses modifyFile to break the .java source code and verify that the error comes out as expected

This commit improves Mill's build failure reporting by:

- Adding failure count tracking to task execution in `Execution`
- Preserving header prefix when updating failure count in `PrefixLogger`
- Adding comprehensive integration tests for failure tracking across different build scenarios
- Ensuring consistent and informative failure reporting in interactive, CI, and multi-threaded environments
@zelosleone zelosleone marked this pull request as draft February 21, 2025 08:39
autofix-ci bot and others added 8 commits February 21, 2025 08:50
This commit improves Mill's logging and failure tracking by:

- Centralizing failure count tracking in the Execution class
- Simplifying PrefixLogger's logging and failure count handling
- Enhancing PromptLogger to better display failure information
- Updating PromptLoggerUtil to render headers with failure counts
- Improving log message formatting and prefix handling
@@ -74,5 +73,467 @@ object FullRunLogsTests extends UtestIntegrationTestSuite {
assert(millProfile.exists(_.obj("label").str == "show"))
assert(millChromeProfile.exists(_.obj("name").str == "show"))
}
test("compilation-error") - integrationTest { tester =>
Copy link
Member

Choose a reason for hiding this comment

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

This test cases is pretty unreadable and there isn't really any chance we'll be able to maintain it going forward. Please follow the style of the test cases above

Copy link
Author

Choose a reason for hiding this comment

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

Yeah no worries, I found the reason of this and will be rewriting it soon.

@zelosleone
Copy link
Author

I will open a new PR to solve the conflicts.

@zelosleone zelosleone closed this Feb 23, 2025
@zelosleone zelosleone deleted the failurecount branch February 24, 2025 16:05
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.

2 participants