Skip to content

ci: [IOPLT-000] Stop validate workflow on failure#8253

Open
LeleDallas wants to merge 1 commit into
masterfrom
IOPLT-000-stop-ci-on-failure
Open

ci: [IOPLT-000] Stop validate workflow on failure#8253
LeleDallas wants to merge 1 commit into
masterfrom
IOPLT-000-stop-ci-on-failure

Conversation

@LeleDallas

Copy link
Copy Markdown
Contributor

Short description

This change was prompted by an issue I noticed in another CI pipeline, where the validation continued executing the remaining targets even after tsc-noemit or lint had already failed.

This pull request introduces a minor update to the validate script to improve CI reliability. The script now runs the affected targets sequentially, ensuring that the validation process stops immediately if any target fails

List of changes proposed in this pull request

  • Updated the validate script to include the --nx-bail flag, causing the process to exit on the first failure during linting, testing, or type checking.

How to test

  • CI should pass
  • Type a linting/type error and run validate script locally, you should see that CI is stopping

Preview

Master IOPLT-000
Screen.Recording.2026-07-08.at.09.56.23.mov
Screen.Recording.2026-07-08.at.09.55.17.mov

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Title Validation for conventional commit type

All good! PR title follows the conventional commit type.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Jira Pull Request Link

This Pull Request refers to Jira issues:

@LazyAfternoons LazyAfternoons left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: is this really an issue? Linting and testing aren't mutually exclusive for example, and it's usually better to get the results of both during a run, especially when pushing by skipping hooks. If the pipeline stops at a lint error, we miss out on knowing whether the tests passed or failed for example. In my opinion this is working as intended and we should not skip other tasks if one of them fails. Let me know.

@LeleDallas

LeleDallas commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Question: is this really an issue? Linting and testing aren't mutually exclusive for example, and it's usually better to get the results of both during a run, especially when pushing by skipping hooks. If the pipeline stops at a lint error, we miss out on knowing whether the tests passed or failed for example. In my opinion this is working as intended and we should not skip other tasks if one of them fails. Let me know.

I see your point, but I think failing fast is more efficient in this case.

If one of the parallel tasks fails (for example, linting fails after 1 minute), there's little value in waiting another 10 minutes for the test suite to complete because the pipeline is already going to fail.
Stopping the remaining tasks as soon as a failure is detected saves CI resources, reduces queue times for everyone, and gives developers faster feedback so they can fix the first blocking issue immediately.

You can always rerun the pipeline after fixing the lint error to validate the tests. In my view, optimizing for quicker feedback and lower CI usage outweighs getting the results of every task in a run that's already known to have failed.

Also, with the current setup, when the CI fails it's not always obvious which job actually failed. The logs are often dominated by the Jest output, making them quite long, so it's easy for developers to assume the tests failed unless they look closely at the status checks at the top. Failing fast would make the root cause much clearer.

@LazyAfternoons

LazyAfternoons commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

If one of the parallel tasks fails (for example, linting fails after 1 minute), there's little value in waiting another 10 minutes for the test suite to complete because the pipeline is already going to fail.

I don't agree with this, unless you see the statics only as a merge gatekeeper. In an hypothetical situation where both linting and testing fails this doesn't really apply as that would require running the statics three times (first time where linting fails, second time where tests fails and a third time where everything pass). With a single full pass you could fix both at once. Of course this is not a everyday situation, however I don't think that we currently have queue times issues (correct me if I'm wrong on this) which this solution could definitely improve.
In a perfect world where you wait for pre comming and pre push hooks, everything should be green in every single push. In an imperfect world where you skip them, having the full view before doing another push is more valuable to me.

Also, with the current setup, when the CI fails it's not always obvious which job actually failed. The logs are often dominated by the Jest output, making them quite long, so it's easy for developers to assume the tests failed unless they look closely at the status checks at the top. Failing fast would make the root cause much clearer.

I think this is a separate issue and shouldn't be factored into whether this change can be helpful.

Of course I'm not opposing this change, just leaving my thoughts.

@LeleDallas

Copy link
Copy Markdown
Contributor Author

In an hypothetical situation where both linting and testing fails this doesn't really apply as that would require running the statics three times (first time where linting fails, second time where tests fails and a third time where everything pass). With a single full pass you could fix both at once.

I see your point, and I agree that in the case where both linting and tests fail, letting everything finish can provide more information in a single run. The scenario you mentioned is a valid one, especially for changes that affect generated definitions, where a single change could potentially break multiple checks.

That said, I think that's the exception rather than the common case. Most CI failures are caused by a single issue, and in those cases fail-fast provides feedback much sooner. If linting fails after a minute, waiting another 10 minutes for the test suite to finish doesn't change the outcome of that pipeline, so we spend CI time and developer waiting time on work whose result can't be acted on until the lint issue is fixed.

Of course this is not a everyday situation, however I don't think that we currently have queue times issues (correct me if I'm wrong on this) which this solution could definitely improve.

It's true that queue times are not currently a problem, but reducing unnecessary CI execution still helps keep turnaround times low as the project and team grow. It's a low-cost optimization that improves feedback speed without changing the validation we perform.
Also, even without queue time issues, there are situations where faster feedback is valuable. For example, if someone needs to open a hotfix PR and pushes a change without being able to run all local checks due to time constraints, having CI fail fast means they get the relevant failure sooner instead of waiting for all jobs to complete before discovering an issue.

In an imperfect world where you skip them, having the full view before doing another push is more valuable to me.

I understand this perspective, but even when multiple failures are reported, it's not guaranteed that fixing them will be completely independent. A change made to fix one issue can affect another check, so a second run may still be needed. For this reason, I think optimizing for the common path (getting actionable feedback as early as possible) is more valuable than optimizing for the less frequent case where multiple independent failures happen at the same time.

I think this is a separate issue and shouldn't be factored into whether this change can be helpful.

I agree with this point. The visibility of failures is a separate concern and is not the motivation for this change. The main goal is reducing unnecessary CI work and shortening the feedback loop.

In any case, thanks for sharing your perspective. I think there are valid arguments on both sides, so maybe it would be useful to discuss this with the whole team and align on the preferred approach for CI behavior

@LazyAfternoons

LazyAfternoons commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

In an hypothetical situation where both linting and testing fails this doesn't really apply as that would require running the statics three times (first time where linting fails, second time where tests fails and a third time where everything pass). With a single full pass you could fix both at once.

I see your point, and I agree that in the case where both linting and tests fail, letting everything finish can provide more information in a single run. The scenario you mentioned is a valid one, especially for changes that affect generated definitions, where a single change could potentially break multiple checks.

That said, I think that's the exception rather than the common case. Most CI failures are caused by a single issue, and in those cases fail-fast provides feedback much sooner. If linting fails after a minute, waiting another 10 minutes for the test suite to finish doesn't change the outcome of that pipeline, so we spend CI time and developer waiting time on work whose result can't be acted on until the lint issue is fixed.

Of course this is not a everyday situation, however I don't think that we currently have queue times issues (correct me if I'm wrong on this) which this solution could definitely improve.

It's true that queue times are not currently a problem, but reducing unnecessary CI execution still helps keep turnaround times low as the project and team grow. It's a low-cost optimization that improves feedback speed without changing the validation we perform. Also, even without queue time issues, there are situations where faster feedback is valuable. For example, if someone needs to open a hotfix PR and pushes a change without being able to run all local checks due to time constraints, having CI fail fast means they get the relevant failure sooner instead of waiting for all jobs to complete before discovering an issue.

In an imperfect world where you skip them, having the full view before doing another push is more valuable to me.

I understand this perspective, but even when multiple failures are reported, it's not guaranteed that fixing them will be completely independent. A change made to fix one issue can affect another check, so a second run may still be needed. For this reason, I think optimizing for the common path (getting actionable feedback as early as possible) is more valuable than optimizing for the less frequent case where multiple independent failures happen at the same time.

I think this is a separate issue and shouldn't be factored into whether this change can be helpful.

I agree with this point. The visibility of failures is a separate concern and is not the motivation for this change. The main goal is reducing unnecessary CI work and shortening the feedback loop.

In any case, thanks for sharing your perspective. I think there are valid arguments on both sides, so maybe it would be useful to discuss this with the whole team and align on the preferred approach for CI behavior

I agree, we can discuss this broadly with the whole team. I think the conversation revolvers around exceptions rather than common cases honestly. I don't see the scenario you are describing as common, however I don't have any data to make any statement. Anecdotally I think the things that actually tend to get skipped or unchecked before pushing are tests.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants