Refactored ADoBuildRunner to accommodate the infrastructure for unit testing.
Introduced a configuration object to track the creation of environment variables in AdoBuildRunner
Added unit tests to test the various scenarios related to AdoApiService methods and added a unit test to test the arguments passed to orchestrator and worker.
Related work items: #2187262
We probe the real file system when doing a cache check and we can see an existing stale opaque output before the opaque directory is materialized from cache and said output is actually deleted. This can cause both cache misses and spurious DFAs.
This PR makes it so during cache lookup (and with lazy materialization enabled) we don't ever cache the existence of files that are under opaque directories.
Related work items: #2185421, #2185423
Previously we detect if a test is a theory when there's a parameter.
If a test is serialize as `TestMe(value: 1)`, then our test framework thinks that it's a theory, and it will run it as
`RUN THEORY TestMe(value: 1)`. The problem with this is, if there are `[InlineData(1)]` and `[InlineData(2)]` for `TestMe`, we will have the following runs:
```
RUN THEORY TestMe(value: 1) =>
RUN TEST TestMe(value: 1)(value: 1)
RUN TEST TestMe(value: 1)(value: 2)
```
Similarly when the serialized test is `TestMe(value: 2)`.
Thus, instead of having 2 tests, we end up having 4 tests.
This PR also reenable QTest.
Related work items: #2195273
Enable QTest for QTest mock UT. This UT needs to be run with QTest so that our ADO QTest integration stage in our release pipeline can succeed.
Related work items: #2195273
Due to bug #2195273 , QTest has been disabled in our selfhost builds. The problem is that we no longer exercise QTest framework in our selfhost, while at the same time we're deploying QTest to customers. Also, because there's no QTest UTs, the (optional) QTest validation stage in our release pipeline failed.
To address the above problem, this PR enables some of light UTs, e.g., UTs that don't have many `[InlineData]` attributes and that don't take a long time to run.
Related work items: #2195273
Add further information about disk performance in PerformanceCollector. To be used for FastDownload statistics in order to help determine where to focus efforts.
- Integrates the 1ES Build Cache resource. Enables consuming the JSON describing the configuration for the resource as the primary description of the shards conforming a blob-base cache topology.
- Relaxes the naming convention policy to allow for containers with arbitrary names. Used to enable CloudBuild to emit SAS tokens for a fixed name.
- Enable using SAS tokens per-container
ObjectPool is backed by a ConcurrentStack. When the ObjectPool returns an item to the pool, it checks the stack length to see if it should hold onto it or not. The implementation of ConcurrentStack.Count is o(n). This change adds parallel tracking with a counter to keep from iterating that stack over and over. This also removes some of the Interlocked calls used for tracking statistics. It is not important that those are always synchronized.
Accessing a pool 1 million times on a 16 core dev machine:
LoopParallelism - number of threads concurrently accessing the pool
PoolSize - number of items in the ObjecPool
| Method | LoopParallelism | PoolSize | Mean | Error | StdDev | Median |
|--------- |---------------- |--------- |----------:|---------:|----------:|----------:|
| Original | 1 | 64 | 30.61 ms | 0.182 ms | 0.171 ms | 30.65 ms |
| Updated | 1 | 64 | 24.19 ms | 0.156 ms | 0.146 ms | 24.25 ms |
| Original | 1 | 512 | 30.48 ms | 0.150 ms | 0.140 ms | 30.56 ms |
| Updated | 1 | 512 | 24.21 ms | 0.162 ms | 0.144 ms | 24.26 ms |
| Original | 32 | 64 | 225.99 ms | 8.697 ms | 25.643 ms | 241.60 ms |
| Updated | 32 | 64 | 190.47 ms | 1.183 ms | 1.106 ms | 190.61 ms |
| Original | 32 | 512 | 224.84 ms | 4.470 ms | 13.040 ms | 227.92 ms |
| Updated | 32 | 512 | 170.82 ms | 5.956 ms | 17.561 ms | 180.02 ms |
| Original | 512 | 64 | 231.52 ms | 4.592 ms | 5.104 ms | 233.16 ms |
| Updated | 512 | 64 | 181.49 ms | 2.409 ms | 2.253 ms | 182.41 ms |
| Original | 512 | 512 | 230.62 ms | 4.596 ms | 9.071 ms | 234.37 ms |
| Updated | 512 | 512 | 122.73 ms | 1.610 ms | 1.427 ms | 122.75 ms |
I doubt this will be revolutionary, but it's a simple change. Snapping to Microsoft.Extensions.ObjectPool would be better, but that's a more involved change.
When building from solution, MSBuild frontend can easily create a path segment that exceeds the segment limit (> 256). This is because the frontend tries to create a unique segment by appending global properties. Even though long paths is enabled, the segment limit cannot be changed.
In this PR, we simply hash the path segment. Due to hashing, the user may need to look at the log file to correlate the path and the pip.
This PR also addresses some issue with path ending with backslash appearing in cmd line. Apparently there's a bug in MSBuild that includes the ending quote as part of the path.
Related work items: #2191024
Temporarily disable QTest due to creating too many UTs.
```csharp
[Theory]
[InlineData(1)]
[InlineData(2)]
[InlineData(3)]
public void TestMe(int data) { ... }
```
Instead of having 3 UTs, you will have 9 UTs.
Due to this Test.BuildXL.FrontEnd.Script.ErrorHandling ends up with 2,224 UTs, and often took > 10 minutes to complete. However, QTest hardcoded the timeout to 10 minutes.
Related work items: #2195273
Gracefully exit workers when released before attachment: we shouldn't fail the worker in this scenario, everything is fine! This happens in 1JS builds quite a bit, and failing the worker is very confusing for the users even if we ignore the failure (an error is logged, etc).
Related work items: #2194383
We track the output directories created by a pip for the sake of stability in probes and directory enumerations. However, we are not tracking the output directories that are created by the sandbox executor as part of the "preparation" it does before executing the tool. These are actual outputs of the pip, created precisely because they are declared as such (directory outputs, opaque roots, parents of statically declared files). So we should track them in the output file system view as "created by the pip" too.
To understand the impact of EventStats missing on orchestrator, sent the eventstats within BuildEndData when orchestrator exit the worker. Worker can log warning message if the EventStats are not matched with its local EventStats.
Related work items: #2160034
- Adds a workaround for the following issue seen on ooui:
- `error DX10105: [Pip62D946C910711FCE, !! ooui - @1js/excel-online [bundle]] PTraceRunner logged the following error: (relative_to_absolute) ['openat'] Could not get path for fd -1 with path ''; errno: 3`
- Both path and dirfd are not set to proper values causing the crash.
- Adds a retry to ReadArgumentLong.
Related work items: #2171844
Run CodeQL only in CodeQL pipeline and enable bug filing. In addition to this PR, a change was made to disable CodeQL in `RunCheckInTests BuildXL PR Validation` pipeline (it's not yaml-based)
We shouldn't process writes in ObservedInputProcessor (OIP shouldn't process writes, only "inputs"), so we should exclude these accesses from the list that we then process. Before this change, observations such as these were subsequently treated as probes by the OIP, and considering such probes in the fingerprint computation could cause spurious cache misses (in particular this happened while working on an orthogonal feature to consider directory probes a bit more carefully).
This change also moves the logic adding the created directories to the `createdDirectoriesMutable` collection so the paths are added before being filtered out (excluded) from the collection of observations.
Julian Bayardo
Lance Collins
Semih Okur
Sahiti Chandramouli
Qi Wang
Marcelo Lynch 🧉
Iman Narasamdya
Michael Pysson
BuildXL-Pipelines
Pasindu Gunasekara 🍣
Oleksii Kononenko
Serge Mera
Paul Ming
Emily Fischer