[rush] Phased builds: test phase has hidden dependency on local build phase
Summary
In the Rushstack monorepo, it gives the appearance that the _phase:build and _phase:test are built in a way that is replicable from cache, but this is not true.
I think that to fix this, we need to be clear up-front what "extra" folders need to be cached, or, we should fix Heft/Rush somehow so that the hidden dependency is resolved.
Repro steps
To reproduce, mimic a situation in which another machine built the project first, and your machine wants to test it.
(This is just the easiest way to reproduce -- in real life, what happens is that 2 builds are racing and whichever wins, the other build gets this error.)
cd apps/heft
echo test >> README.md
rush build --to .
git clean -ffdx -e node_modules
rush install
rush test --to .
Expected result: Rush loads the build phase for heft from cache, and successfully runs unit tests.
Actual result:
--[ FAILURE: @rushstack/heft (test) ]------------------------[ 2.75 seconds ]--
Error: Could not find the Jest TypeScript data file at "/Users/enelson/dev/rushstack2/apps/heft/.heft/build-cache/heft-jest-data.json". Was the compiler invoked?
Details
According to the spec for phased builds, we'd want the output of the build phase to be cacheable into discrete folders -- I'm not sure if .heft/build-cache is such a folder, or if testing data would also get written there, or if it could be moved into temp/something.
Standard questions
Please answer these questions to help us investigate your issue more quickly:
| Question | Answer |
|---|---|
@microsoft/rush globally installed version? |
5.70.0 |
rushVersion from rush.json? |
5.70.0 |
useWorkspaces from rush.json? |
yes |
| Operating system? | mac |
| Would you consider contributing a PR? | Yes (if I knew what change to make) |
Node.js version (node -v)? |
14.19.2 |
@dmichon-msft @D4N14L Do you cache files under the .heft/ folder?
@elliot-nelson where would the fix go? In Heft? In Rush? In the config files for this monorepo?
@octogonz ^ Here's an example of a fix that works for Rushstack and for HBO's monorepo (which I used to test it).
To clarify, you don't have to change anything, you can just modify the configuration. In HBO's monorepo, for example, I tested with these settings:
{
"operationSettings": [
// Old (pre-phased)
{
"operationName": "build",
"outputFolderNames": ["lib", "lib-commonjs", "dist", "temp"]
},
// New (post-phased)
{
"operationName": "_phase:build",
"outputFolderNames": ["lib", "lib-commonjs", "dist", "temp/heft"]
},
{
"operationName": "_phase:test",
"outputFolderNames": ["temp/coverage", "temp/jest-reports"]
}
]
}
You can fix the issue by changing temp/heft to .heft, without making any changes to Heft. So, the suggested PR is not urgent, but I do think it lines up the output folders more nicely.
Internally, yes, we do cache files under .heft folder for builds. This allows us to reuse the build cache from TypeScript as well as the output from the Jest plugin when running during the build. I agree that this file doesn't really belong there, however I'm unsure if the file should be cached at all. The file contains runtime-specific information that won't necessarily be true on another run (for example, the skipTimestampCheck property is populated based on whether or not you are running in --watch mode).
I think the correct answer here is probably to write it into the temp folder during the test stage (instead of during the build stage as it currently runs). That's the approach I've taken in the multi-phase Heft revamp: https://github.com/D4N14L/rushstack/blob/dc26f85ef54a26130b55c9bd716713304df6ae6a/heft-plugins/heft-jest-plugin/src/JestPlugin.ts#L489 ... Unfortunately, that makes more sense in multi-phase Heft than it does for current Heft, since it would mean you'd have to teach the external Jest plugin to load the TypeScript plugin configuration file, and the TypeScript plugin is currently bundled with Heft.
Ideally, we would avoid caching it at all, since it's intended to be temporary. On that note, I find it odd that we're caching the temp folder at all... really anything "temporary" should be wiped out on every clean/re-created on every execution. Personally, I think it makes sense to keep caches that will be used/reused in future runs under the .heft folder, in plugin-specific subfolders (ex. .heft/typescript-cache for TypeScript, .heft/jest-cache for Jest, etc.), while allowing temp to be used as a folder to store execution-specific/ephemeral information. It would be even better if these paths (the cache path and the temp path) for a plugin could be provided via the HeftSession, to allow for plugins to target known-safe locations to write their stuff into.
Lemme know your thoughts!
Thanks @D4N14L! At least internally, we ended up landing on the temp/ folder because it was a convenient place to tell all projects "don't add a bunch of crazy gitignore folders, if your tooling produces stuff, stick it in temp/".
So for example, we configure Jest to put junit and coverage reports in temp/, we configure AWS CDK to put stuff in temp/cdk.out instead of ./cdk.out, and so on (for tools that allow configuration, anyway).
But, we still want to cache the output of test phases: for example, let's say you have a PR Build in Azure DevOps, most PRs are going to pull 90% of the test phase from cache (because you don't want to run tests for projects that were unaffected by the PR). However, you still want the cached coverage report from the last successful run of the project, otherwise your full-repo code coverage report makes no sense from build to build.
I guess if it's just a naming change, I'd happily rename temp/coverage to .heft/coverage, but it no longer works as a universal bucket -- e.g. for the few projects in the repo that don't use Heft, they still need a "place to dump stuff", and in some cases, that stuff makes sense to cache as an output.
I should add here, we've successfully moved to phased builds internally, and just adding the .heft/ folder as an output folder for the _phase:build phase worked fine.
So, a potential easy patch for now is to just recommend that to anyone looking to implement phased builds -- no big changes are necessary to get phased builds working properly (although they may still make sense as part of the roadmap).
I'm closing this for now... the fix was relatively simple, which is to add .heft to your output folders.
This may have a different solution after the new heft revamp.