Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conditionally run golden_tests_harvester for run_impeller_golden_tests. #51325

Merged
zanderso merged 1 commit into from
Mar 11, 2024
Merged

Conditionally run golden_tests_harvester for run_impeller_golden_tests. #51325

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions testing/run_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,16 @@ def run_impeller_golden_tests(build_dir: str):
print(diff_result.stdout.decode())
raise RuntimeError('impeller_golden_tests diff failure')

# On release builds and local builds, we typically do not have GOLDCTL set,
# which on other words means that this invoking the SkiaGoldClient would
# throw. Skip this step in those cases and log a notice.
if 'GOLDCTL' not in os.environ:
Copy link
Member

@gaaclarke gaaclarke Mar 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this to the golden tests harvester instead? Having to know about this environment at this level is kind of leaky. Someone has to consider its usage at 3 different levels ([run_tests, harvester, skia gold client]). It's best if it could be found in skiagoldclient, better if it could be in golden tests harvester, worst is adding knowledge about it here.

Copy link
Contributor Author

@matanlurey matanlurey Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add it to the harvester, but I think we have to answer flutter/flutter#144948. In other words, I think it's leaky/confusing behavior for the harvester to pretend to execute, and it's better to either (a) not run it at all, or (b) make not running explicit (i.e. --dry-run).

print_divider('<')
print(
'Skipping the SkiaGoldClient invocation as the GOLDCTL environment variable is not set.'
)
return

with DirectoryChange(harvester_path):
bin_path = Path('.').joinpath('bin').joinpath('golden_tests_harvester.dart')
run_cmd([dart_bin, 'run', str(bin_path), temp_dir])
Expand Down