Build Scan parsing is updated to read values when previously unknown

[erichaagdev]

Overview

This PR removes the build_cache_metrics_only and all_data values passed to and from various functions to control what summary data is parsed. The goal of this is to improve code readability by having one less variable to keep track of in the call stack.

The approach I decided to take was to set summary values only if they were empty. In the case of local experiments, a lot of the environmental summary values are already known. This approach prefers values determined by the scripts over those obtained from a Build Scan. Below is a summary of the changes:

• Previously, these fields were only set when all_data was passed to the parse function. Now, they are set when the current value is empty:
  • base_urls
  • build_scan_urls
  • git_repos
  • git_branches
  • git_commit_ids
  • requested_tasks
  • build_outcomes
• Previously, these fields were only set when all_data was passed to the parse function. Now they are always set:
  • remote_build_cache_urls
  • remote_build_cache_shards

Testing

In order to test these changes, I compared the output before and after my changes. Below is an overview of the results. You will also see changes from #397 which is why the fetch logging is different.

Test results

Gradle - Experiment 3 (Before | After)

---

Gradle - Experiment 3 - No Gradle Enterprise API Access (Before | After)

---

Gradle - Experiment 4 (Before | After)

---

Gradle - Experiment 5 (Before | After)

---

Maven - Experiment 2 (Before | After)

---

Maven - Experiment 2 - No Gradle Enterprise API Access (Before | After)

---

Maven - Experiment 3 (Before | After)

---

Maven - Experiment 4 (Before | After)

[etiennestuder]

@erichaagdev in the case of the CI/local experiment, I would have expected that we print more details about the CI build scan when we fetch it the first time (similar to CI/CI).

Hm, I guess the extra output for getting the CI build the first time would be earlier in the output. Maybe it is there but just not visible in the screenshot.

[erichaagdev]

Hm, I guess the extra output for getting the CI build the first time would be earlier in the output. Maybe it is there but just not visible in the screenshot.

Correct. Let me rebase this branch onto erichaagdev/fix-brief-logging and then I will run the cross platform tests. There you can inspect the full output.

[erichaagdev]

Successful cross platform tests run: https://github.com/gradle/gradle-enterprise-build-validation-scripts/actions/runs/4674139330

@etiennestuder here you can see the full output for CI/Local: https://github.com/gradle/gradle-enterprise-build-validation-scripts/actions/runs/4674139330/jobs/8278073687#step:11:12

[etiennestuder]

Successful cross platform tests run: https://github.com/gradle/gradle-enterprise-build-validation-scripts/actions/runs/4674139330

@etiennestuder here you can see the full output for CI/Local: https://github.com/gradle/gradle-enterprise-build-validation-scripts/actions/runs/4674139330/jobs/8278073687#step:11:12

Nice. LGTM

[jthurne]

All of my comments are more about ideas for further improvements. The change itself looks good.

[erichaagdev]

@jthurne I went forward with the one suggestion you made. If we want to rename the brief and verbose parameters I can do that in a subsequent PR.

Merging this one as it has been approved.