invertase / melos

๐ŸŒ‹ A tool for managing Dart projects with multiple packages. With IntelliJ and Vscode IDE support. Supports automated versioning, changelogs & publishing via Conventional Commits.
https://melos.invertase.dev/~melos-latest
Apache License 2.0
1.13k stars 201 forks source link

fix: exec command with failFast should fail immediately #665

Closed MohiuddinM closed 6 months ago

MohiuddinM commented 6 months ago

Description

The expected behavior of failFast option is to fail the whole exec command as soon as a single fail occurs, and it works as expected if concurrency=1. But if concurrency > 1, it diverges from the expected behavior and instead keeps running until all pooled jobs have finished executing.

This PR fixes this divergence, and the exec command will fail immediately on a single fail irrespective of concurrency value.

Type of Change

MohiuddinM commented 6 months ago

Note to maintainers: only the last commit is related, others are showing because I didn't fork specifically for this fix, and instead merged this into my fork.

MohiuddinM commented 6 months ago

@spydon I have:

MohiuddinM commented 6 months ago

Tests are flaky I guess because all 3 are being run at the same time, and some times "a" fails first, and "b", "c" are cancelled. Otherwise, some other fails. I need to find a way to add delay before "exit 1" so it becomes more predictable.

   Expected: String ignoring Ansii '$ melos exec\n'
              '  โ””> exit 1\n'
              '     โ””> RUNNING (in 3 packages)\n'
              '\n'
              '--------------------------------------------------------------------------------\n'
              '--------------------------------------------------------------------------------\n'
              '\n'
              '$ melos exec\n'
              '  โ””> exit 1\n'
              '     โ””> FAILED (in 1 packages)\n'
              '        โ””> c (with exit code 1)\n'
              '     โ””> CANCELED (in 2 packages)\n'
              '        โ””> a (due to failFast)\n'
              '        โ””> b (due to failFast)\n'
              ''
    Actual: '$ melos exec\n'
              '  โ””> exit 1\n'
              '     โ””> RUNNING (in 3 packages)\n'
              '\n'
              '--------------------------------------------------------------------------------\n'
              '--------------------------------------------------------------------------------\n'
              '\n'
              '$ melos exec\n'
              '  โ””> exit 1\n'
              '     โ””> FAILED (in 1 packages)\n'
              '        โ””> b (with exit code 1)\n'
              '     โ””> CANCELED (in 2 packages)\n'
              '        โ””> a (due to failFast)\n'
              '        โ””> c (due to failFast)\n'
MohiuddinM commented 6 months ago

@spydon I found a solution to the random failure order by adding delays (see: https://github.com/invertase/melos/pull/665/commits/1ab3177051b704a199f7db8b1e86e91acc0c9c08).

The test failing now is unrelated I don't know why, but it passes on my PC.