make sure version check includes hotfixes #33459
Conversation
goderbauer
added
a: tests
gspencergoog
left a comment
gspencergoog
left a comment
There was a problem hiding this comment.
I've never seen the v1.4.9-hotfix.1-6-gdeadbeef or v1.4.9-hotfix.1-gcafefood format versions before. What are they for? And are the parts after the "hotfix.1" really required (in your regex you had them as required).
dev/bots/test.dart
Outdated
| } | ||
| final RegExp pattern = RegExp(r'^[0-9]+\.[0-9]+\.[0-9]+(-pre\.[0-9]+)?$'); | ||
| if (!version.contains(pattern)) { | ||
| final RegExp hotfixPattern = RegExp(r'^v([0-9]+)\.([0-9]+)\.([0-9]+)(?:\+hotfix\.([0-9]+))?-([0-9]+)-g([a-f0-9]+)$'); |
There was a problem hiding this comment.
Use \d for digits, add uppercase for hex digits, and you don' t need most of the groups, since you don't ever read the groups.
| final RegExp hotfixPattern = RegExp(r'^v([0-9]+)\.([0-9]+)\.([0-9]+)(?:\+hotfix\.([0-9]+))?-([0-9]+)-g([a-f0-9]+)$'); | |
| final RegExp hotfixPattern = RegExp(r'^v\d+\.\d+\.\d+(-hotfix\.\d+)?(-\d+)?(-g[a-fA-F0-9]+)?$'); |
dev/bots/test.dart
Outdated
| print('$redLine'); | ||
| exit(1); | ||
| } | ||
| final RegExp pattern = RegExp(r'^[0-9]+\.[0-9]+\.[0-9]+(-pre\.[0-9]+)?$'); |
There was a problem hiding this comment.
Doesn't this also need the v at the beginning? Or should the other one not have the v?
| final RegExp pattern = RegExp(r'^[0-9]+\.[0-9]+\.[0-9]+(-pre\.[0-9]+)?$'); | |
| final RegExp pattern = RegExp(r'^\d+\.\d+\.\d+(-pre\.\d+)?$'); |
|
Also, it would be pretty awesome to have some tests here: that's a pretty complex pair of You can also combine them into one: they share a lot of commonality. |
|
The possible version strings (as string literals except
I do not believe anything else should ever match the patterns here. I believe this is a complete regexp that matches this:
|
Co-Authored-By: Greg Spencer <[email protected]>
dev/bots/test.dart
Outdated
| exit(1); | ||
| } | ||
| final RegExp pattern = RegExp(r'^[0-9]+\.[0-9]+\.[0-9]+(-pre\.[0-9]+)?$'); | ||
| final RegExp pattern = RegExp(r'\d+\.\d+\.\d+(?:|-pre\.\d+|\+hotfix\.\d+)'); |
There was a problem hiding this comment.
do we need ^ at the start and $ at the end?
|
I know this file isn't really tested but if you could find a way to add tests to this that would make me feel more comfortable with the increasing complexity of this regexp. In any case, |
* master: (25 commits) Increase daemon protocol version for getSupportedPlatforms (flutter#33980) skip web test on crazy import (flutter#34017) Compatibility pass on flutter/foundation tests for JavaScript compilation. (1) (flutter#33349) 0602dbb Roll src/third_party/dart 9dcb026b26..6e0d978505 (72 commits) (flutter#34027) Add chrome stable to dockerfile and web shard (flutter#33787) Codegen an entrypoint for flutter web applications (flutter#33956) Revert "Reland "Text inline widgets, TextSpan rework" (flutter#33946)" (flutter#34002) Revert "Re-add deprecated method for plugin migration compatibility. (flutter#34006)" (flutter#34022) Remove print (flutter#34004) Manual roll the engine to land the timing API (flutter#33989) Make plugins Swift-first on macOS (flutter#33997) Re-add deprecated method for plugin migration compatibility. (flutter#34006) make sure version check includes hotfixes (flutter#33459) Respond to AndroidView focus events. (flutter#33901) Add 'doctor' support for Windows (flutter#33872) Add use_frameworks to macOS Podfile template (flutter#33987) [Material] Create a themable Range Slider (continuous and discrete) (flutter#31681) Updating names to correct versioning convention (flutter#33865) Whitelist adb.exe heap corruption exit code. (flutter#33951) [flutter_tool] Fix 'q' for Fuchsia profile/debug mode (flutter#33846) ...
5 participants
Description
Make sure our version test knows about hotfixes, the only way I know how.
Related Issues
#31880