Allow the LocalBuildJobRunner to continue on task failure

[pmachapman]

Fixes #1020

I was able to recreate this locally by stopping the mongo-1 docker container, then restarting it after a couple of minutes.

---

This change is 

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.56%. Comparing base (7ca6d4e) to head (30917c9).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1022      +/-   ##
==========================================
+ Coverage   72.54%   72.56%   +0.02%     
==========================================
  Files         391      391              
  Lines       21627    21653      +26     
  Branches     2815     2818       +3     
==========================================
+ Hits        15689    15713      +24     
- Misses       4812     4814       +2     
  Partials     1126     1126              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ddaspit]

@ddaspit reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Enkidu93 and pmachapman).

---

src/Machine/src/Serval.Machine.Shared/Services/LocalBuildJobRunner.cs line 116 at r1 (raw file):

                // If one of the tasks has faulted, cancel the other
                if (completedTask.IsFaulted)

Is this scenario testable?

[Enkidu93]

@Enkidu93 reviewed all commit messages and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on pmachapman).

[pmachapman]

@pmachapman made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on ddaspit and Enkidu93).

---

src/Machine/src/Serval.Machine.Shared/Services/LocalBuildJobRunner.cs line 116 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Is this scenario testable?

@ddaspit I have added a unit test, and a FaultedTranslationEngineLocalBuildJobRunner that allow this scenario to be tested. Please let me know if this is OK.

I know the Task.Delay is a bit icky - BackgroundService doesn't really offer much to debug issues in ExecuteAsync. Probably the only other way I can think of is to refactor the LocalBuildJobRunner to make testing easier (or maybe a custom Serval BackgroundService class), but that will add a lot of boilerplate I think just for tests.

I believe the delay should be sufficient given there is no I/O going on in the service, and nearly dependency that is injected is either mocked or empty.

[ddaspit]

@ddaspit reviewed 2 files and all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on Enkidu93).

---

src/Machine/src/Serval.Machine.Shared/Services/LocalBuildJobRunner.cs line 116 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

@ddaspit I have added a unit test, and a FaultedTranslationEngineLocalBuildJobRunner that allow this scenario to be tested. Please let me know if this is OK.

I know the Task.Delay is a bit icky - BackgroundService doesn't really offer much to debug issues in ExecuteAsync. Probably the only other way I can think of is to refactor the LocalBuildJobRunner to make testing easier (or maybe a custom Serval BackgroundService class), but that will add a lot of boilerplate I think just for tests.

I believe the delay should be sufficient given there is no I/O going on in the service, and nearly dependency that is injected is either mocked or empty.

The Task.Delay makes me a little nervous. If the test doesn't end up being stable, we can make it more robust at that point.