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 failed tasks count to Mill's progress indicator #4617

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zelosleone
Copy link

@zelosleone zelosleone commented Feb 24, 2025

  • Add failed tasks count to Mill's progress indicator

This change introduces a mechanism to track and display the number of failed tasks during build execution. Key modifications include:

  • Added setFailedTasksCount method to the Logger trait
  • Updated Execution to count and report failed tasks
  • Modified PromptLogger to include failed tasks count in the progress indicator
  • Implemented setFailedTasksCount in MultiLogger and PromptLogger

fixes #4588

The new feature provides more visibility into build status by showing the number of failed tasks in the progress line, helping users quickly identify build issues.

Example output from the integration test:

[3346] public class Foo{
[3346]     public static String generateHtml(String text){
[3346]         Context context = new Context();
[3346]         context.setVariable("text", text);
[3346]         return new TemplateEngine().process("<h1 th:text=\"${text}\"></h1>", context);
[3346]     }
[3346]     public static void main(String[] args) {
[3346]         ArgumentParser parser = ArgumentParsers.newFor("template").build()
[3346]                 .defaultHelp(true)
[3346]                 .description("Inserts text into a HTML template");
[3346]
[3346]         parser.addArgument("-t", "--text")
[3346]                 .required(true)
[3346]                 .help("text to insert");
[3346]
[3346]         Namespace ns = null;
[3346]         try {
[3346]             ns = parser.parseArgs(args);
[3346]         }catch(Exception e){
[3346]             System.out.println(e.getMessage());
[3346]             System.exit(1);
[3346]         }
[3346]
[3346]         System.out.println(generateHtml(ns.getString("text")));
[3346]     }
[3346] }
[3346] '
[3346] [DEBUG] failedTasksCounter - modified content: 'package foo;
[3346]
[3346] import net.sourceforge.argparse4j.ArgumentParsers;
[3346] import net.sourceforge.argparse4j.inf.ArgumentParser;
[3346] import net.sourceforge.argparse4j.inf.ArgumentParserException;
[3346] import net.sourceforge.argparse4j.inf.Namespace;
[3346] import org.thymeleaf.TemplateEngine;
[3346] import org.thymeleaf.context.Context;
[3346]
[3346] public class Foo {{
[3346]     public static String generateHtml(String text){
[3346]         Context context = new Context();
[3346]         context.setVariable("text", text);
[3346]         return new TemplateEngine().process("<h1 th:text=\"${text}\"></h1>", context);
[3346]     }
[3346]     public static void main(String[] args) {
[3346]         ArgumentParser parser = ArgumentParsers.newFor("template").build()
[3346]                 .defaultHelp(true)
[3346]                 .description("Inserts text into a HTML template");
[3346]
[3346]         parser.addArgument("-t", "--text")
[3346]                 .required(true)
[3346]                 .help("text to insert");
[3346]
[3346]         Namespace ns = null;
[3346]         try {
[3346]             ns = parser.parseArgs(args);
[3346]         }catch(Exception e){
[3346]             System.out.println(e.getMessage());
[3346]             System.exit(1);
[3346]         }
[3346]
[3346]         System.out.println(generateHtml(ns.getString("text")));
[3346]     }
[3346] }
[3346] '
[3346] [DEBUG] failedTasksCounter - failed build isSuccess: false
[3346] [DEBUG] failedTasksCounter - failed build out: ''
[3346] [DEBUG] failedTasksCounter - failed build err: '============================== compile ==============================
[3346] [30/30] compile
[3346] [30] [info] compiling 1 Java source to C:\Users\deniz\Documents\GitHub\mill\out\integration\feature\full-run-logs\local\fork\test.dest\sandbox\run-4\out\compile.dest\classes ...
[3346] [30] [error] C:/Users/deniz/Documents/GitHub/mill/out/integration/feature/full-run-logs/local/fork/test.dest/sandbox/run-4/src/foo/Foo.java:11:5: illegal start of expression
[3346] [30/30, 1 failed] ============================== compile ============================== 4s
[3346] 1 tasks failed
[3346] compile javac returned non-zero exit code'
[3346] [DEBUG] failedTasksCounter - pattern matches: true
[3346] [DEBUG] failedTasksCounter - matches found: List([30/30, 1 failed])

* Add failed tasks count to Mill's progress indicator

This change introduces a mechanism to track and display the number of failed tasks during build execution. Key modifications include:

- Added `setFailedTasksCount` method to the `Logger` trait
- Updated `Execution` to count and report failed tasks
- Modified `PromptLogger` to include failed tasks count in the progress indicator
- Implemented `setFailedTasksCount` in `MultiLogger` and `PromptLogger`

The new feature provides more visibility into build status by showing the number of failed tasks in the progress line, helping users quickly identify build issues.
@zelosleone
Copy link
Author

@lihaoyi solved the issue and provided an integration test just like you wanted, included the debug info just in case to not to take your time for nothing.

For people who use windows and search through github to see how to run the integration tests:
$env:MILL_TEST_RESOURCE_DIR="$pwd\integration\feature\full-run-logs\resources"; $env:MILL_INTEGRATION_LAUNCHER="$pwd\mill.bat"; $env:MILL_INTEGRATION_SERVER_MODE="true"; .\mill.bat integration.feature[full-run-logs].local.fork.test

as one-liner command on powershell works.

@lihaoyi
Copy link
Member

lihaoyi commented Feb 25, 2025

Seems to partially work now! Still hitting some issues:

lihaoyi mill$ /Users/lihaoyi/Github/mill/mill-assembly.jar main.test.compile        
Mill version SNAPSHOT is different than configured for this directory!
Configured version is 0.12.8-1-46e216-native (/Users/lihaoyi/Github/mill/.config/mill-version)
[1002/1006] testkit.compile
[1002] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/out/testkit/compile.dest/classes ...
[1002] [error] -- [E103] Syntax Error: /Users/lihaoyi/Github/mill/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala:24:0 
[1002] [error] 24 |breakage
[1002] [error]    |^^^^^^^^
[1002] [error]    |Illegal start of toplevel definition
[1002] [error]    |
[1002] [error]    | longer explanation available when compiling with `-explain`
[1002] [error] one error found
[1006/1006] ============================== main.test.compile ==============================
1 tasks failed
testkit.compile Compilation failed
lihaoyi mill$ /Users/lihaoyi/Github/mill/mill-assembly.jar -k main.test.compile
Mill version SNAPSHOT is different than configured for this directory!
Configured version is 0.12.8-1-46e216-native (/Users/lihaoyi/Github/mill/.config/mill-version)
[1002/1006] testkit.compile
[1002] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/out/testkit/compile.dest/classes ...
[1002] [error] -- [E103] Syntax Error: /Users/lihaoyi/Github/mill/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala:24:0 
[1002] [error] 24 |breakage
[1002] [error]    |^^^^^^^^
[1002] [error]    |Illegal start of toplevel definition
[1002] [error]    |
[1002] [error]    | longer explanation available when compiling with `-explain`
[1002] [error] one error found
[1002] [1002] testkit.compile failed
[1004/1006] 
[1003/1006] 
[1004] [1004] main.test.upstreamCompileOutput failed
[1003] [1003] main.test.transitiveCompileClasspath failed
[1005/1006] 
[1005] [1005] main.test.compileClasspath failed
[1006/1006] 
[1006] [1006] main.test.compile failed
[1006/1006, 4 failed] ============================== main.test.compile ==============================
1 tasks failed
testkit.compile Compilation failed
  • If I don't pass in -k, the n failures message does not properly show up
  • When I do pass in -k it seems to count all the downstream tasks that were skipped as failures, even though only one task actually failed.

@zelosleone
Copy link
Author

Seems to partially work now! Still hitting some issues:

lihaoyi mill$ /Users/lihaoyi/Github/mill/mill-assembly.jar main.test.compile        
Mill version SNAPSHOT is different than configured for this directory!
Configured version is 0.12.8-1-46e216-native (/Users/lihaoyi/Github/mill/.config/mill-version)
[1002/1006] testkit.compile
[1002] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/out/testkit/compile.dest/classes ...
[1002] [error] -- [E103] Syntax Error: /Users/lihaoyi/Github/mill/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala:24:0 
[1002] [error] 24 |breakage
[1002] [error]    |^^^^^^^^
[1002] [error]    |Illegal start of toplevel definition
[1002] [error]    |
[1002] [error]    | longer explanation available when compiling with `-explain`
[1002] [error] one error found
[1006/1006] ============================== main.test.compile ==============================
1 tasks failed
testkit.compile Compilation failed
lihaoyi mill$ /Users/lihaoyi/Github/mill/mill-assembly.jar -k main.test.compile
Mill version SNAPSHOT is different than configured for this directory!
Configured version is 0.12.8-1-46e216-native (/Users/lihaoyi/Github/mill/.config/mill-version)
[1002/1006] testkit.compile
[1002] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/out/testkit/compile.dest/classes ...
[1002] [error] -- [E103] Syntax Error: /Users/lihaoyi/Github/mill/testkit/src/mill/testkit/UtestIntegrationTestSuite.scala:24:0 
[1002] [error] 24 |breakage
[1002] [error]    |^^^^^^^^
[1002] [error]    |Illegal start of toplevel definition
[1002] [error]    |
[1002] [error]    | longer explanation available when compiling with `-explain`
[1002] [error] one error found
[1002] [1002] testkit.compile failed
[1004/1006] 
[1003/1006] 
[1004] [1004] main.test.upstreamCompileOutput failed
[1003] [1003] main.test.transitiveCompileClasspath failed
[1005/1006] 
[1005] [1005] main.test.compileClasspath failed
[1006/1006] 
[1006] [1006] main.test.compile failed
[1006/1006, 4 failed] ============================== main.test.compile ==============================
1 tasks failed
testkit.compile Compilation failed
* If I don't pass in `-k`, the `n failures` message does not properly show up

* When I do pass in `-k` it seems to count all the downstream tasks that were skipped as failures, even though only one task actually failed.

Ups! That actually has an incredibly easy fix, i will post it with the updated integration test.

This change improves the accuracy of failed tasks counting in Mill's build execution:

- Updated `Execution` to correctly count failed tasks using `asFailing` instead of `asSuccess`
- Extended test coverage for failed tasks counting in different scenarios
- Added tests for both default and keep-going (-k) modes
- Verified that only actual task failures are counted, not skipped tasks
- Demonstrated handling of complex dependency structures with task failures
@zelosleone
Copy link
Author

@lihaoyi Should be fixed now, thanks for pointing it out. I forgot to test the skipped tasks.

@lihaoyi
Copy link
Member

lihaoyi commented Feb 25, 2025

I just tried the latest 631fa95, still not seeing the n failures message appear reliably when I break things locally and run main.__.compile. How are you testing this manually?

@zelosleone
Copy link
Author

I just tried the latest 631fa95, still not seeing the n failures message appear reliably when I break things locally and run main.__.compile. How are you testing this manually?

Via the integration test and debug info i get, can you share your testing files so i can try with them instead?

@lihaoyi
Copy link
Member

lihaoyi commented Feb 25, 2025

Here's what I'm running to exercise this end-to-end

git clean -xdf
./mill dist.installLocal
echo breakage >> main/test/src/mill/TestMain.scala
ci/patch-mill-bootstrap.sh
./mill-assembly.jar main.__.test

The final command shows the failure in the logs but not in the prompt

lihaoyi mill$ ./mill-assembly.jar main.__.test
Mill version SNAPSHOT is different than configured for this directory!
Configured version is 0.12.8-1-46e216-native (/Users/lihaoyi/Github/mill/.config/mill-version)
[mill-build/build.mill-62/66] compile
[mill-build/build.mill-62] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/out/mill-build/mill-build/compile.dest/classes ...
[mill-build/build.mill-62] [info] done compiling
[build.mill-62/66] compile
[build.mill-62] [info] compiling 29 Scala sources to /Users/lihaoyi/Github/mill/out/mill-build/compile.dest/classes ...
[build.mill-62] [warn] -- /Users/lihaoyi/Github/mill/website/package.mill:316:13
[build.mill-62] [warn] 316 │            millLastTag.split('.'),
[build.mill-62] [warn]     │            ^
[build.mill-62] [warn]     │method copyArrayToImmutableIndexedSeq in class LowPriorityImplicits2 is deprecated since 2.13.0: implicit conversions from Array to immutable.IndexedSeq are implemented by copying; use `toIndexedSeq` explicitly if you want to copy, or use the more efficient non-copying ArraySeq.unsafeWrapArray
[build.mill-62] [warn] one warning found
[build.mill-62] [info] done compiling
[2727/2985] main.test.compile
[2727] [info] compiling 4 Scala sources to /Users/lihaoyi/Github/mill/out/main/test/compile.dest/classes ...
[2727] [error] -- [E103] Syntax Error: /Users/lihaoyi/Github/mill/main/test/src/mill/TestMain.scala:6:0 
[2727] [error] 6 |breakage
[2727] [error]   |^^^^^^^^
[2727] [error]   |Illegal start of toplevel definition
[2727] [error]   |
[2727] [error]   | longer explanation available when compiling with `-explain`
[2727] [error] one error found
[2985/2985] ============================== main.__.test ============================== 26s
1 tasks failed
main.test.compile Compilation failed

@zelosleone zelosleone marked this pull request as draft February 25, 2025 01:43
@zelosleone
Copy link
Author

Here's what I'm running to exercise this end-to-end

git clean -xdf
./mill dist.installLocal
echo breakage >> main/test/src/mill/TestMain.scala
ci/patch-mill-bootstrap.sh
./mill-assembly.jar main.__.test

The final command shows the failure in the logs but not in the prompt

lihaoyi mill$ ./mill-assembly.jar main.__.test
Mill version SNAPSHOT is different than configured for this directory!
Configured version is 0.12.8-1-46e216-native (/Users/lihaoyi/Github/mill/.config/mill-version)
[mill-build/build.mill-62/66] compile
[mill-build/build.mill-62] [info] compiling 1 Scala source to /Users/lihaoyi/Github/mill/out/mill-build/mill-build/compile.dest/classes ...
[mill-build/build.mill-62] [info] done compiling
[build.mill-62/66] compile
[build.mill-62] [info] compiling 29 Scala sources to /Users/lihaoyi/Github/mill/out/mill-build/compile.dest/classes ...
[build.mill-62] [warn] -- /Users/lihaoyi/Github/mill/website/package.mill:316:13
[build.mill-62] [warn] 316 │            millLastTag.split('.'),
[build.mill-62] [warn]     │            ^
[build.mill-62] [warn]     │method copyArrayToImmutableIndexedSeq in class LowPriorityImplicits2 is deprecated since 2.13.0: implicit conversions from Array to immutable.IndexedSeq are implemented by copying; use `toIndexedSeq` explicitly if you want to copy, or use the more efficient non-copying ArraySeq.unsafeWrapArray
[build.mill-62] [warn] one warning found
[build.mill-62] [info] done compiling
[2727/2985] main.test.compile
[2727] [info] compiling 4 Scala sources to /Users/lihaoyi/Github/mill/out/main/test/compile.dest/classes ...
[2727] [error] -- [E103] Syntax Error: /Users/lihaoyi/Github/mill/main/test/src/mill/TestMain.scala:6:0 
[2727] [error] 6 |breakage
[2727] [error]   |^^^^^^^^
[2727] [error]   |Illegal start of toplevel definition
[2727] [error]   |
[2727] [error]   | longer explanation available when compiling with `-explain`
[2727] [error] one error found
[2985/2985] ============================== main.__.test ============================== 26s
1 tasks failed
main.test.compile Compilation failed

Will check with your method, converted to draft to stop the running time of build to avoid wasting it, will ping you when i solve it

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.

Make PromptLogger show the count of failed tasks (500USD Bounty)
2 participants