Make codesign.dart integration test easier to run locally
#50289
Conversation
…ndly to run from the command line
fluttergithubbot
added
the
c: contributor-productivity
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
This change is a test. |
| 'find', | ||
| <String>[ | ||
| cacheDirectory, | ||
| rootDirectory, |
There was a problem hiding this comment.
Shouldn't we default this to something non-null? Or make it a non-optional parameter and assert it's not null?
There was a problem hiding this comment.
good catch. i had a fallback default, forget why I deleted it...
dev/bots/codesign.dart
Outdated
|
|
||
| for (final String binaryPath in findBinaryPaths()) { | ||
| if (!Platform.isMacOS) { | ||
| print('Warning! This script only works on macOS, as it requires Xcode.'); |
There was a problem hiding this comment.
uber-nit: on CI, this might read strangely if you thought it was running on macOS, and it might be helpful to just print out Platform.operatingSystem in here.
Ideally we'd also want to make sure the binaries we need (e.g. codesign) are on the path, but it's probably fine to skip that for this script.
There was a problem hiding this comment.
updated error message.
| final String flutterRepoRoot = path.normalize(path.join(path.dirname(Platform.script.path), '..', '..')); | ||
| return path.normalize(path.join(flutterRepoRoot, 'bin', 'cache')); | ||
| } | ||
| String get repoRoot => path.normalize(path.join(path.dirname(Platform.script.toFilePath()), '..', '..')); |
There was a problem hiding this comment.
the previous of this script actually failed on cirrus because the space in the script file path was escaped. changing Platform.script.path -> Platform.script.toFilePath() fixed that.
4 participants
Add two checks before running test to make this script more user-friendly to run locally from the command line.