alexeyinkin commented on code in PR #26978:
URL: https://github.com/apache/beam/pull/26978#discussion_r1218966647
##########
learning/tour-of-beam/frontend/integration_test/tour_page_test.dart:
##########
@@ -125,6 +132,30 @@ Future<void> _checkUnitContentLoadsProperly(
);
}
+Future<void> _checkSdkChangesProperly(WidgetTester wt, UnitModel node) async {
+ final defaultSdk = _getTourNotifier(wt).playgroundController.sdk;
+ final sdkCache = GetIt.instance.get<SdkCache>();
+ String? previousId;
+ for (final sdk in sdkCache.getSdks()) {
+ await _setSdk(sdk.title, wt);
+ final actualId =
+ _getTourNotifier(wt).playgroundController.selectedExample?.path;
+ expect(actualId, isNot(previousId));
Review Comment:
This is too weak. Test the SDK of the newly loaded unit or example.
##########
learning/tour-of-beam/frontend/integration_test/tour_page_test.dart:
##########
@@ -100,6 +102,11 @@ Future<void> _checkNode(UnitModel node, WidgetTester wt)
async {
);
await _checkUnitContentLoadsProperly(node, wt);
+
+ final hasSnippet = _getTourNotifier(wt).isUnitContainsSnippet;
+ if (!_isSdkChangingChecked && hasSnippet) {
+ await _checkSdkChangesProperly(wt, node);
+ }
Review Comment:
People would not expect `_checkNode` to go into such extra work depending on
the global state. And if it breaks, it will be confusing, because it has
nothing to do with 'check node'.
Move this to a separate function on the same level as other globals.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
