[rush] build-cache ignores allowWarningsInSuccessfulBuild

[relm923]

Summary

The new build-cache system does not write to the cache when the command writes to STDERR even when allowWarningsInSuccessfulBuild: true is used in the command config.

This is easiest to see when running rush test and using jest since Jest regularly writes to STDERR (jestjs/jest#5064). But will occur with any command that writes to STDERR

I would expect the build-cache logic to respect the allowWarningsInSuccessfulBuild so we can still take advantage of caching even when third parties use STDERR incorrectly.

Repro steps

Add global command that triggers something to write to STDERR

// command-line.json
{
  "name": "test",
  "commandKind": "bulk",
  "summary": "This command invokes Jest to test apps.",
  "safeForSimultaneousRushProcesses": true,
  "allowWarningsInSuccessfulBuild": true,
  "incremental": true,
  "enableParallelism": true
},

// package.json
"scripts": {
  "test": "jest"
},

Expected result:

1. Run command rush test. No cache hits - all tests run (printing warnings) and cache is updated
2. Run command rush test again. All cache hits - no tests run

Actual result:

1. Run command rush test. No cache hits - all tests run (printing warnings) and cache is NOT updated
2. Run command rush test again. No cache hits - all tests run (printing warnings) and cache is NOT updated

Details

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question	Answer
@microsoft/rush globally installed version?	5.47.0
rushVersion from rush.json?	5.47.0
useWorkspaces from rush.json?	true
Operating system?	Mac
Would you consider contributing a PR?	Yes
Node.js version (node -v)?	v16.3.0

[relm923]

I'm not very familiar with this logic but it seems like

rushstack/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts

Line 315 in 78aa5ad

} else if (hasWarningOrError) {

needs to updated to respect allowWarningsInSuccessfulBuild

[octogonz]

I would expect the build-cache logic to respect the allowWarningsInSuccessfulBuild so we can still take advantage of caching even when third parties use STDERR incorrectly.

Agreed, this seems like a bug.

@iclanton

[Measy]

It seems related #1402

[relm923]

@octogonz @iclanton any update on this?

I'm happy to help contribute a fix but will likely need some guidance getting started

[F3n67u]

@relm923 Try let this if also pass when allowWarningsInSuccessfulBuild & status === TaskStatus.SuccessWithWarning is true.

rushstack/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts

Line 327 in 78aa5ad

if (status === TaskStatus.Success && projectBuildDeps) {

[F3n67u]

I have encountered the same issue.

[octogonz]

I'm happy to help contribute a fix but will likely need some guidance getting started

That would be great! If you have questions about the approach, #contributor-helpline is the quickest way to get unblocked.

[relm923]

@octogonz took a stab at a PR (#2835) let me know what you think