You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/10/03 00:06:14 UTC
[GitHub] [druid] ccaominh opened a new pull request #10469: Test UI to trigger auto compaction
ccaominh opened a new pull request #10469:
URL: https://github.com/apache/druid/pull/10469
### Description
In the web console end-to-end tests, use the new UI to trigger auto compaction instead of calling the REST API directly so that the UI is covered by tests.
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added end-to-end tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #10469: Test UI to trigger auto compaction
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #10469:
URL: https://github.com/apache/druid/pull/10469#discussion_r499192514
##########
File path: web-console/e2e-tests/component/datasources/overview.ts
##########
@@ -116,6 +124,23 @@ export class DatasourcesOverview {
const editActions = await this.page.$$('span[icon=wrench]');
editActions[index].click();
+ await this.waitForPopupMenu();
+ }
+
+ private async waitForPopupMenu(): Promise<void> {
+ await this.page.waitFor(1000); // https://blueprintjs.com/docs/#core/components/popover.testing
Review comment:
After incorporating the changes with the upgraded playwright version, it seems this fixed sleep is no longer needed to make the E2E test reliable, so I removed it.
Although it may not be needed for E2E tests, I think your suggestion to have a testing mode for components with `Popover`s could be useful for unit tests though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #10469: Test UI to trigger auto compaction
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10469:
URL: https://github.com/apache/druid/pull/10469#discussion_r499157234
##########
File path: web-console/e2e-tests/component/datasources/overview.ts
##########
@@ -116,6 +124,23 @@ export class DatasourcesOverview {
const editActions = await this.page.$$('span[icon=wrench]');
editActions[index].click();
+ await this.waitForPopupMenu();
+ }
+
+ private async waitForPopupMenu(): Promise<void> {
+ await this.page.waitFor(1000); // https://blueprintjs.com/docs/#core/components/popover.testing
Review comment:
Do you think that we should make an allowance in the UI and have a 'testing' mode that zeroes out all the delays in all the popovers in the UI `hoverCloseDelay={0} hoverOpenDelay={0}`. I am imagining that you could evaluate some sort of `enterTestingMode()` function in the app context and it will do this and other things. Not for this PR but wanted to get your thoughts on it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky merged pull request #10469: Test UI to trigger auto compaction
Posted by GitBox <gi...@apache.org>.
vogievetsky merged pull request #10469:
URL: https://github.com/apache/druid/pull/10469
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on pull request #10469: Test UI to trigger auto compaction
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #10469:
URL: https://github.com/apache/druid/pull/10469#issuecomment-703121688
Looks like some of your `waitFor`s need to become `waitForSelector`s given your previous PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org