Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Playground integration test run and editing #25033

Merged

Conversation

Malarg
Copy link
Contributor

@Malarg Malarg commented Jan 17, 2023

Playground integration test run and editing

Resolves #24936
Resolves #24923
Resolves #24922
Resolves #24921
Resolves #24937
Resolves #24938
Resolves #24939
Resolves #24940
Resolves #24941
Resolves #25051
Resolves #25052
Resolves #25053
Resolves #25278

resolves #25329
resolves #25331
resolves #25336
resolves #25332
resolves #25333


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI.

alexeyinkin and others added 14 commits December 20, 2022 17:56
* Integration test for changing SDK and running code (apache#24779)

* Rename an integration test (apache#24779)

* Use enum to switch SDK in integration test (apache#24779)

* Find SDK in a dropdown by key (apache#24779)

* Add a TODO (apache#24779)

* Fix exports (apache#24779)
* Integration test for changing SDK and running code (apache#24779)

* Rename an integration test (apache#24779)

* Use enum to switch SDK in integration test (apache#24779)

* Find SDK in a dropdown by key (apache#24779)

* Add a TODO (apache#24779)

* Fix exports (apache#24779)

* Integration tests miscellaneous UI (#383)

* miscellaneous ui integration tests

* reverted pubspec.lock

* gradle tasks ordered alhpabetically

* integration tests refactoring

* clean code

* integration tests miscellaneous ui fix pr

* rename method

* added layout adaptivity

* A minor cleanup (apache#24779)

Co-authored-by: Dmitry Repin <[email protected]>
final startSource = controller.source;
await wt.enterText(find.codeField(), 'print("Hello World!');

expect(startSource != controller.source, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(startSource != controller.source, true);
expect(controller.source, isNot(startSource));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


expect(startSource != controller.source, true);

await wt.runShortcut(controller.resetShortcut.shortcuts.keys);
Copy link
Contributor

Choose a reason for hiding this comment

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

Make shortcuts OS-dependent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Check, please

Copy link
Contributor

Choose a reason for hiding this comment

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

All 5 shortcuts work in Ubuntu.

@@ -33,6 +36,13 @@ import 'package:playground_components/src/widgets/drag_handle.dart';
import 'package:playground_components_dev/playground_components_dev.dart';

extension CommonFindersExtension on CommonFinders {
Finder appDropdownButtonWithText(String text) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Look for PipelineOptionsDropdown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

await wt.tapAndSettle(find.runOrCancelButton());

final playgroundController = wt.findPlaygroundController();
expect(playgroundController.outputResult, contains('Pipeline cancelled'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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


_checkIfRawTextCorrect('--some test --some2 test2');

await wt.tapAndSettle(find.pipelineOptionsOptionsTab());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await wt.tapAndSettle(find.pipelineOptionsOptionsTab());
await wt.tapAndSettle(find.pipelineOptionsListTab());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

);
}

Future<void> _editingAndResettingChanges(WidgetTester wt) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Split to 2 methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

expect(playgroundController.source, equals(code));
}

Future<void> _checkCodeHighlighting(WidgetTester wt) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Future<void> _checkCodeHighlighting(WidgetTester wt) async {
Future<void> _expectMoreThanOneColor(WidgetTester wt) async {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Finder foldToggles = _getFoldToggles();

Finder topToggle =
wt.getCenter(foldToggles.at(0)).dy < wt.getCenter(foldToggles.at(1)).dy
Copy link
Contributor

Choose a reason for hiding this comment

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

Look for inputs the same way, comparing their centers, remove keys from inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 52 to 53
await wt.tapAndSettle(find.exampleSelector());
await wt.tapAndSettle(find.exampleSelector());
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

Comment on lines +38 to +39
await _checkFilteringExamplesByTags(wt);
await _checkFilteringExamplesBySearchString(wt);
Copy link
Contributor

Choose a reason for hiding this comment

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

Check the actual examples before filtering and after filtering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

tasks.register("integrationTest_standalone_cancel_running_example") {
runIntegrationTest("standalone_cancel_running_example", "/")
Copy link
Contributor

Choose a reason for hiding this comment

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

doLast everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

duration,
);
} catch (e) {
//ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Explain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 35 to 41
final source = wt
.findPlaygroundController()
.snippetEditingController
?.activeFileController
?.codeController
.fullText ??
'';
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if there is a getter for this. It may be on the Copy button of the embedded playground.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

(WidgetTester wt) async {
await init(wt);

await wt.tapAndSettle(find.appDropdownButtonWithText('Pipeline Options'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await wt.tapAndSettle(find.appDropdownButtonWithText('Pipeline Options'));
await wt.openPipelineOptions();

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

await wt.tap(find.runOrCancelButton());
await Future.delayed(const Duration(milliseconds: 300));

await wt.tapAndSettle(find.runOrCancelButton());
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment that cancelling is just for speed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 100 to 102
if (examples.isEmpty) {
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete or explain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted


return categories
.map((e) => e.examples.length)
.reduce((value, element) => value + element);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.reduce((value, element) => value + element);
.reduce((a, b) => a + b);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Thanks

notifyListeners();
}

void showAutocompleter() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void showAutocompleter() {
void showSuggestions() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -32,18 +32,28 @@ const kTimeoutErrorText =
const kUnknownErrorText =
'Something went wrong. Please try again later or create a GitHub issue';
const kProcessingStartedText = 'The processing has started\n';
const kProcessingStartedOptionsText =
'The processing has started with pipeline options: ';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'The processing has started with pipeline options: ';
'The processing has been started with the pipeline options: ';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


import 'native_platform.dart';

extension LogicalKeyboardKeyExtension on LogicalKeyboardKey {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
extension LogicalKeyboardKeyExtension on LogicalKeyboardKey {
class LogicalKeyboardKeyExtension {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -92,6 +88,10 @@ extension CommonFindersExtension on CommonFinders {
return byType(MoreActions);
}

Finder pipelineOptions() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Finder pipelineOptions() {
Finder pipelineOptionsDropdown() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Finder verticallyAt(int index, WidgetTester wt) =>
_alignedIndexAt(index, Axis.vertical, wt);

Finder _alignedIndexAt(int index, Axis axis, WidgetTester wt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nthOnAxis or atIndexOnAxis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 64 to 70
if (a == b) {
return 0;
} else if (a > b) {
return 1;
} else {
return -1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

return (a - b).sign.toInt();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

//ignore
}

await wt.wait(duration);
Copy link
Contributor

Choose a reason for hiding this comment

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

Investigate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

This worked for me instead, and the test is green:

  await wt.pump();
  await Future.delayed(duration);

Can this be used instead of this function? If not, then why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this approach freezes main isolate (watch the animation on the button with both approaches)

Copy link
Contributor

Choose a reason for hiding this comment

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

To me this is a lesser evil than inventing a method with a vague name without being sure how exactly it does what it does. We can just test the result.

Copy link
Contributor

@alexeyinkin alexeyinkin Jan 31, 2023

Choose a reason for hiding this comment

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

What is the definition of "freeze"?

I believe it is impossible to put the thread to sleep in JavaScript.
I guess what appears "frozen" is just animations not ticking or ticking but not rendering.

If pump() ticks animations once and schedules a render, then for the test purpose there technically is no difference between calling it once vs calling it in a loop, it's just that with the loop we are visually satisfied with smooth animation at the cost of active waiting and complexity. I suggest keeping things simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added pumpAndSettleNoException

@alexeyinkin
Copy link
Contributor

A successful run of this test in GitHub workflow: https://github.com/apache/beam/actions/runs/4172006277/jobs/7222584726

Copy link
Contributor

@damondouglas damondouglas left a comment

Choose a reason for hiding this comment

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

@pabloem LGTM

@pabloem pabloem merged commit 48c143d into apache:master Mar 9, 2023
@pabloem
Copy link
Member

pabloem commented Mar 9, 2023

lgtm thanks y'all!

ruslan-ikhsan pushed a commit to akvelon/beam that referenced this pull request Mar 10, 2023
* Integration test to load the default example of the default SDK and change the example (apache#24730) (apache#24729)

* Fix formatting and README (apache#24730)

* Support collection v1.17.0 (apache#24730)

* LoadingIndicator on chaning examples, remove duplicating licenses (apache#24730)

* Add a missing license header (apache#24730)

* Integration test for changing SDK and running code (apache#24779) (#382)

* Integration test for changing SDK and running code (apache#24779)

* Rename an integration test (apache#24779)

* Use enum to switch SDK in integration test (apache#24779)

* Find SDK in a dropdown by key (apache#24779)

* Add a TODO (apache#24779)

* Fix exports (apache#24779)

* Issue24779 integration changing sdk from 24370 (#387)

* Integration test for changing SDK and running code (apache#24779)

* Rename an integration test (apache#24779)

* Use enum to switch SDK in integration test (apache#24779)

* Find SDK in a dropdown by key (apache#24779)

* Add a TODO (apache#24779)

* Fix exports (apache#24779)

* Integration tests miscellaneous UI (#383)

* miscellaneous ui integration tests

* reverted pubspec.lock

* gradle tasks ordered alhpabetically

* integration tests refactoring

* clean code

* integration tests miscellaneous ui fix pr

* rename method

* added layout adaptivity

* A minor cleanup (apache#24779)

Co-authored-by: Dmitry Repin <[email protected]>

* integration tests run and editing

* example selector test

* minor fixes

* rat

* fix pr

* minor

* minor

* rat

* integration test finder written

* integration test minor fixes

* minor fixes

* removed comment

* minor fixes

* playground integration tests minor fixes

* integration test pumpAnSettleNoException

* integration test shortcut refactor

* integration test another changing shortcuts running

* upgrade to flutter 3.7.1

* workaround comment

* playground frontend updated major versions

* issues 25329 25331 25336

* 25329 extract connectivity extension to separate file

* Upgrade Flutter to 3.7.3 in integration tests (apache#24730)

* Fix integration test (apache#24730)

* fix cors issue and added mouse scroll to tags

* Upgrade Flutter in Dockerfile (apache#24720)

* sorting moved to model

* sorting moved to model

* sorting moved to model

* bugs fix

* issue 25278

* fix pr

* quites fix in en.yaml

* Fix not loading default example (apache#25528)

* fix compile error

* Refactor output tabs, test embedded playground (apache#25136) (#439)

* Refactor output tabs, test embedded playground (apache#25136)

* Clean up (apache#25136)

* Change example paths to IDs in integration tests

* Panning the graph tab (apache#25118)

---------

Co-authored-by: Alexey Inkin <[email protected]>
Co-authored-by: alexeyinkin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment