[page_objects/common_page] convert to ts #50771

dmlemeshko · 2019-11-15T13:26:29Z

Summary

Covert common_page.js to ts and simplify the logic

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-11-15T14:36:49Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 8372210

elasticmachine · 2019-11-15T16:52:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a545eb6

elasticmachine · 2019-11-15T18:51:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8404b54

dmlemeshko · 2019-11-15T19:30:52Z

test/functional/page_objects/common_page.ts

+    private async loginIfPrompted(appUrl: string) {
+      let currentUrl = await browser.getCurrentUrl();
+      log.debug(`currentUrl = ${currentUrl}\n    appUrl = ${appUrl}`);
+      await find.byCssSelector('[data-test-subj="kibanaChrome"]', 6 * defaultFindTimeout); // 60 sec waiting


I decreased timeout from 4 min to 1 min because it makes sense to fail and retry from scratch with url navigation rather than wait so long for the page to be loaded

Curious, why private?

Well, it is used only in the same class as a helper function. For the tests we expose navigation functions, e.g. navigateToApp and use { shouldLoginIfPrompted: boolean } to manage login. I'm open to change it, but not sure we want it to be used directly

dmlemeshko · 2019-11-15T19:33:48Z

test/functional/page_objects/common_page.ts

        });
      }

      log.debug('navigating to ' + appName + ' url: ' + appUrl);

-      function navigateTo(url) {
-        return retry.try(function () {
+      await retry.tryForTime(defaultTryTimeout * 2, async () => {


I decreased retry timeout from 6 min to 4 min since it should be fairly enough time for navigation + login, in case of failure we should get a screenshot with stack trace instead of mocha timeout

dmlemeshko · 2019-11-15T19:35:12Z

test/functional/page_objects/common_page.ts

          // since we're using hash URLs, always reload first to force re-render
-          return kibanaServer.uiSettings.getDefaultIndex()


I removed this line since we didn't use the return result. Not sure what was the purpose?

There was code here at one time that checked if the default index was set when we tried to navigate to Discover, Visualize, Dashboard (but not Management). If you didn't have it set, it would set it to logstash-* and log a warning message. Maybe we fixed any tests which weren't setting the default index, in which case that code isn't needed any more.

LeeDr · 2019-11-15T19:38:40Z

test/functional/page_objects/common_page.ts

+
+          if (!navSuccessful) {
+            const msg =
+              'App failed to load: ' +


Is it required to break this up into so many lines :-(

A template literal would make this much more readable.

Agree, I will fix it :)

fixed in a362ca3

elasticmachine · 2019-11-15T22:16:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a362ca3

dmlemeshko · 2019-11-15T22:52:43Z

retest

elasticmachine · 2019-11-16T00:10:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a362ca3

dmlemeshko · 2019-11-16T17:55:59Z

retest

elasticmachine · 2019-11-16T19:38:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a362ca3

dmlemeshko · 2019-11-18T09:59:20Z

@elasticmachine merge upstream

elasticmachine · 2019-11-18T11:17:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b839229

mistic

Changes LGTM in the perspective of the operations team code owners files

wayneseymour · 2019-11-18T16:17:42Z

test/functional/page_objects/common_page.ts

+          // Browsers don't show the ':port' if it's 80 or 443 so we have to
+          // remove that part so we can get a match in the tests.
+          const navSuccessful = new RegExp(
+            appUrl.replace(':80/', '/').replace(':443/', '/') +


Could be shorter (just less code to read)

Suggested change

appUrl.replace(':80/', '/').replace(':443/', '/') +

appUrl.replace(/:80\/|:443\//, '/') +

@dmlemeshko If you have to re-push, maybe this suggestion is useful.

Cool @wayneseymour, I didn't notice it. I will push next time :)

LeeDr

LGTM

* [page_objects/common_page] convert to ts * fix lint errors * descrease navigation timeouts * use template literal for log messages

…-fallback * 'master' of github.com:elastic/kibana: (116 commits) [Maps] move apply global filter settting from layer to source (elastic#50523) [SIEM] Fix: Empty `Source` / `Destination` shown when only ports are populated (elastic#50843) [Maps] Delay vector tile layer syncing until spritesheet is loaded (elastic#48955) [Maps] prevent users from overflowing URL when filtering by shape (elastic#50747) [DOCS] Mark Beats central management as discontinued (elastic#49423) [page_objects/common_page] convert to ts (elastic#50771) [NP Kibana Migrations ] kibana plugin home (elastic#50444) [DOCS] Shareables naming convention (elastic#50497) [ML] DF Analytics - auto-populate model_memory_limit (elastic#50714) Increase alerting test stability and reduce flakiness (elastic#50246) [ML] Remaning new_job_new folder (elastic#50917) [Telemetry] Show opt-in changes for OSS users (elastic#50831) [ML] Fix lat_long anomalies table links menu and value formatting (elastic#50916) [Dev] Fix serialising a really big string (elastic#50915) Better explanation about the Prettier recommendation (extension vs. NPM module) (elastic#50629) [Monitoring] Use a basic monitoring user for tests (elastic#47865) [Monitoring] Gracefully handle issue with filebeat indices (elastic#48929) [Monitoring] Improve permissions required around setup mode (elastic#50421) Additional validation for elasticsearch username (elastic#48247) Revert changes to use_kibana_ui_setting (elastic#50877) ... # Conflicts: # src/legacy/core_plugins/console/server/request.test.ts

* [page_objects/common_page] convert to ts * fix lint errors * descrease navigation timeouts * use template literal for log messages

[page_objects/common_page] convert to ts

8372210

fix lint errors

a545eb6

descrease navigation timeouts

8404b54

dmlemeshko added release_note:skip Skip the PR/issue when compiling release notes test_ui_functional test_xpack_functional v7.5.1 v7.6.0 v8.0.0 labels Nov 15, 2019

dmlemeshko marked this pull request as ready for review November 15, 2019 19:26

dmlemeshko requested a review from a team as a code owner November 15, 2019 19:26

dmlemeshko requested review from LeeDr and wayneseymour November 15, 2019 19:27

dmlemeshko commented Nov 15, 2019

View reviewed changes

LeeDr reviewed Nov 15, 2019

View reviewed changes

use template literal for log messages

a362ca3

Merge branch 'master' into ftr/ts-common_page

b839229

dmlemeshko requested review from tylersmalley and LeeDr November 18, 2019 09:59

mistic approved these changes Nov 18, 2019

View reviewed changes

wayneseymour reviewed Nov 18, 2019

View reviewed changes

wayneseymour approved these changes Nov 18, 2019

View reviewed changes

jbudz approved these changes Nov 18, 2019

View reviewed changes

LeeDr approved these changes Nov 18, 2019

View reviewed changes

dmlemeshko merged commit 6bc43bb into elastic:master Nov 18, 2019

dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Nov 18, 2019

[page_objects/common_page] convert to ts (elastic#50771)

54b5286

* [page_objects/common_page] convert to ts * fix lint errors * descrease navigation timeouts * use template literal for log messages

dmlemeshko mentioned this pull request Nov 18, 2019

[7.x] [page_objects/common_page] convert to ts (#50771) #50968

Merged

dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Nov 18, 2019

[page_objects/common_page] convert to ts (elastic#50771)

d973cad

* [page_objects/common_page] convert to ts * fix lint errors * descrease navigation timeouts * use template literal for log messages

dmlemeshko mentioned this pull request Nov 18, 2019

[7.5] [page_objects/common_page] convert to ts (#50771) #50969

Merged

dmlemeshko added a commit that referenced this pull request Nov 18, 2019

[page_objects/common_page] convert to ts (#50771) (#50968)

79da1e2

* [page_objects/common_page] convert to ts * fix lint errors * descrease navigation timeouts * use template literal for log messages

dmlemeshko added a commit that referenced this pull request Nov 18, 2019

[page_objects/common_page] convert to ts (#50771) (#50969)

caf02e5

* [page_objects/common_page] convert to ts * fix lint errors * descrease navigation timeouts * use template literal for log messages

dmlemeshko deleted the ftr/ts-common_page branch March 24, 2020 07:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[page_objects/common_page] convert to ts #50771

[page_objects/common_page] convert to ts #50771

dmlemeshko commented Nov 15, 2019

elasticmachine commented Nov 15, 2019

elasticmachine commented Nov 15, 2019

elasticmachine commented Nov 15, 2019

dmlemeshko Nov 15, 2019

wayneseymour Nov 18, 2019

dmlemeshko Nov 18, 2019 •

edited

Loading

dmlemeshko Nov 15, 2019

dmlemeshko Nov 15, 2019

LeeDr Nov 18, 2019

LeeDr Nov 15, 2019

tylersmalley Nov 15, 2019

dmlemeshko Nov 15, 2019

dmlemeshko Nov 15, 2019

elasticmachine commented Nov 15, 2019

dmlemeshko commented Nov 15, 2019

elasticmachine commented Nov 16, 2019

dmlemeshko commented Nov 16, 2019

elasticmachine commented Nov 16, 2019

dmlemeshko commented Nov 18, 2019

elasticmachine commented Nov 18, 2019

mistic left a comment

wayneseymour Nov 18, 2019

wayneseymour Nov 18, 2019

dmlemeshko Nov 18, 2019

LeeDr left a comment

		// since we're using hash URLs, always reload first to force re-render
		return kibanaServer.uiSettings.getDefaultIndex()

	appUrl.replace(':80/', '/').replace(':443/', '/') +
	appUrl.replace(/:80\/\|:443\//, '/') +

[page_objects/common_page] convert to ts #50771

[page_objects/common_page] convert to ts #50771

Conversation

dmlemeshko commented Nov 15, 2019

Summary

Checklist

For maintainers

elasticmachine commented Nov 15, 2019

💔 Build Failed

elasticmachine commented Nov 15, 2019

💚 Build Succeeded

elasticmachine commented Nov 15, 2019

💚 Build Succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmlemeshko Nov 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 15, 2019

💚 Build Succeeded

dmlemeshko commented Nov 15, 2019

elasticmachine commented Nov 16, 2019

💚 Build Succeeded

dmlemeshko commented Nov 16, 2019

elasticmachine commented Nov 16, 2019

💚 Build Succeeded

dmlemeshko commented Nov 18, 2019

elasticmachine commented Nov 18, 2019

💚 Build Succeeded

mistic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeeDr left a comment

Choose a reason for hiding this comment

dmlemeshko Nov 18, 2019 •

edited

Loading