Functional tests for the Getting Started page #11850

ycombinator · 2017-05-17T12:24:22Z

This PR starts to add some functional tests for the Getting Started page. Specifically, it adds functional tests that exercise the scenarios under which a user should or should not get redirected to the Getting Started page.

It also does a small bit of refactoring in the Console tests that use the Getting Started page object to opt out of the Getting Started page.

chrisronline · 2017-05-17T13:43:55Z

test/functional/page_objects/getting_started_page.js

  class GettingStartedPage {
    async clickOptOutLink() {
      log.debug('Clicking opt-out link');
      await testSubjects.click('lnkGettingStartedOptOut');
    }
+
+    async optOutLinkExists() {
+      return await testSubjects.exists('lnkGettingStartedOptOut');


Super minor, but this seems the only method that doesn't have a log.debug

chrisronline · 2017-05-17T13:51:23Z

test/functional/page_objects/getting_started_page.js


  const log = getService('log');
  const testSubjects = getService('testSubjects');

+  const PageObjects = getPageObjects(['common']);
+
  class GettingStartedPage {


Why is this even an instantiated class? I'm not seeing any local state or anything. All the methods just call static methods, or instance methods off already instantiated objects.

I'm not sure TBH. I was following the pattern already established with other page objects. If we want to change this pattern I suggest we open a new issue for doing it across the board for all page objects and then fix it there.

@chrisronline created #11863

chrisronline · 2017-05-17T13:53:38Z

test/functional/apps/management/_getting_started.js

+        // delete .kibana index and update configDoc
+        await kibanaServer.uiSettings.replace({
+          'dateFormat:tz':'UTC',
+          'defaultIndex':'logstash-*'


Maybe not a better way, but does this defaultIndex need to match anything else that is setup in the test runner? I'd just want to avoid a scenario where this value is configured somewhere, it's copied here, but then someone possibly changes the value later in the future and this test breaks and it's not clear why. It's entirely possible that I'm missing something but just wanted to point it out in case it could happen

Yeah, I see what you mean. There's an implicit dependency here between this line and the await esArchiver.load('discover'); line a couple of lines further down. It would probably be better to create a wrapper method somewhere like loadDefaultIndex that loads the actual data using esArchiver.load but also sets up the defaultIndex value, and then use that wrapper method here.

All that said, this here is a pattern that is already established and being used in several places. So I suggest we open an issue for fixing this across the board rather than increasing the scope of this PR.

@chrisronline created #11865

chrisronline · 2017-05-17T13:55:10Z

test/functional/apps/management/_getting_started.js

+
+  const PageObjects = getPageObjects(['common', 'gettingStarted']);
+
+  describe('Getting Started page', () => {


Should we have a test that ensures the navigation is displayed properly when the user manually navigates to the Getting Starting page from the Management page?

I added this as part of 5351e61.

spalger · 2017-05-17T23:22:03Z

test/functional/apps/console/_console.js

@@ -19,9 +19,9 @@ export default function ({ getService, getPageObjects }) {

  describe('console app', function describeIndexTests() {
    before(async function () {
+      await PageObjects.gettingStarted.optOut();


spalger · 2017-05-17T23:41:56Z

test/functional/apps/management/_getting_started.js

+        await kibanaServer.uiSettings.replace({});
+      });
+
+      describe('when user has not opted out of Getting Started page', () => {


You might want to add a before hook here that ensures that the getting started local storage entry is not set, with remote.deleteLocalStorageItem() https://theintern.github.io/leadfoot/module-leadfoot_Command.html#deleteLocalStorageItem

When I try this I get this error:

│ [DELETE http://localhost:9515/session/dfa8779e7e55a011f085086a3a75031b/local_storage/key/kibana.isGettingStartedOptedOut] <unknown>: Failed to read the 'localStorage' property from'Window': Access is denied for this document. │ (Session info: chrome=58.0.3029.110) │ (Driver info: chromedriver=2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=Mac OS X 10.12.4 x86_64)

I think its because the Kibana web site hasn't been loaded up at that point.

I think I have a fix.

Fixed as part of 6334019.

ycombinator · 2017-05-19T11:42:58Z

@chrisronline @spalger I've addressed review comments and rebased after #11881 was merged recently. This is ready for your 👀 again. Thanks!

chrisronline

LGTM. Just some minor notes

chrisronline · 2017-05-19T13:59:01Z

test/functional/page_objects/common_page.js

@@ -79,23 +79,28 @@ export function CommonPageProvider({ getService, getPageObjects, getPageObject }

            if (loginPage && !wantedLoginPage) {
              log.debug(`Found loginPage username = ${config.get('servers.kibana.username')}`);
-              await PageObjects.shield.login(
+              return PageObjects.shield.login(


What's the reason behind these changes? Do you need the checking to live in a promise chain?

Hmm, these changes don't look right. They are probably a result of this PR co-existing with #11881 at one point and all the rebasing that was happening at the time. I'm going to update this PR with only commits related to it. Stay tuned!

ycombinator · 2017-05-22T22:27:51Z

Jenkins, test this

spalger

LGTM after https://github.com/elastic/kibana/pull/11850/files#diff-83fb3c25a2e44e2dc982d59d38b0241eR22 is removed

ycombinator · 2017-05-22T22:42:51Z

Thanks, I've removed that line in 5089559. Will merge once CI goes green.

This set of tests specifically tests the scenarios under which a user should or should not get redirected to the Getting Started page

…d page

* Adding some more functionality to the Getting Started page object * Using optOut method from Getting Started page object * Adding functional tests for Getting Started page This set of tests specifically tests the scenarios under which a user should or should not get redirected to the Getting Started page * Adding log.debug message * Conforming to HTML style guide * Using new GettingStarted page object methods + opting out * Adding test for nav being shown * Removing unnecessary line * Navigate to Discover expecting to be redirected to the Getting Started page * Trying beforeEach instead of before * Remove LS data index + load empty kibana index * Removing unnecessary line * Fixing order of operations

ycombinator · 2017-05-23T14:09:52Z

Backported to:

5.x / 5.5: ae16cb3

* Adding some more functionality to the Getting Started page object * Using optOut method from Getting Started page object * Adding functional tests for Getting Started page This set of tests specifically tests the scenarios under which a user should or should not get redirected to the Getting Started page * Adding log.debug message * Conforming to HTML style guide * Using new GettingStarted page object methods + opting out * Adding test for nav being shown * Removing unnecessary line * Navigate to Discover expecting to be redirected to the Getting Started page * Trying beforeEach instead of before * Remove LS data index + load empty kibana index * Removing unnecessary line * Fixing order of operations

This reverts commit 099178a.

* Revert "When on an embedded page, bypass Getting Started gate check (#12040)" This reverts commit 05293f1. * Revert "Making tweaks. (#12003)" This reverts commit aa3fa06. * Revert "Functional tests for the Getting Started page (#11850)" This reverts commit 099178a. * Revert "Prevent flicker on Getting Started page (#11826)" This reverts commit c4b3ade. * Revert "Getting Started page (#11805)" This reverts commit 32eff37. * Remove check for Getting Started page from navigateToApp

* Revert "When on an embedded page, bypass Getting Started gate check (elastic#12040)" This reverts commit 05293f1. * Revert "Making tweaks. (elastic#12003)" This reverts commit aa3fa06. * Revert "Functional tests for the Getting Started page (elastic#11850)" This reverts commit 099178a. * Revert "Prevent flicker on Getting Started page (elastic#11826)" This reverts commit c4b3ade. * Revert "Getting Started page (elastic#11805)" This reverts commit 32eff37. * Remove check for Getting Started page from navigateToApp

ycombinator added :Management review v5.5.0 v6.0.0 labels May 17, 2017

ycombinator requested a review from spalger May 17, 2017 12:24

ycombinator mentioned this pull request May 17, 2017

Prevent flicker on Getting Started page #11826

Merged

ycombinator requested review from epixa and chrisronline and removed request for epixa May 17, 2017 12:27

chrisronline reviewed May 17, 2017

View reviewed changes

epixa added the v6.0.0-alpha2 label May 17, 2017

spalger reviewed May 17, 2017

View reviewed changes

ycombinator mentioned this pull request May 17, 2017

[tests/functional] opt out of getting started on navigation #11881

Merged

spalger reviewed May 17, 2017

View reviewed changes

ycombinator force-pushed the getting-started/functional-tests branch 2 times, most recently from 5351e61 to 111c9d7 Compare May 19, 2017 11:42

chrisronline approved these changes May 19, 2017

View reviewed changes

ycombinator force-pushed the getting-started/functional-tests branch 2 times, most recently from 7cf3ee7 to 36ce0eb Compare May 19, 2017 14:26

spalger approved these changes May 22, 2017

View reviewed changes

ycombinator added 7 commits May 22, 2017 17:00

Adding some more functionality to the Getting Started page object

d56e3e0

Using optOut method from Getting Started page object

a1a450b

Adding functional tests for Getting Started page

2e30345

This set of tests specifically tests the scenarios under which a user should or should not get redirected to the Getting Started page

Adding log.debug message

6d53363

Conforming to HTML style guide

25b8e04

Using new GettingStarted page object methods + opting out

b7bad51

Adding test for nav being shown

e52932b

Removing unnecessary line

5089559

ycombinator force-pushed the getting-started/functional-tests branch from 83460ae to 5089559 Compare May 23, 2017 00:00

ycombinator added 5 commits May 22, 2017 17:30

Navigate to Discover expecting to be redirected to the Getting Starte…

8ba24c3

…d page

Trying beforeEach instead of before

649d885

Remove LS data index + load empty kibana index

1dfdef3

Removing unnecessary line

9cb122a

Fixing order of operations

0807666

ycombinator merged commit 099178a into elastic:master May 23, 2017

ycombinator deleted the getting-started/functional-tests branch May 23, 2017 14:09

ycombinator added a commit to ycombinator/kibana that referenced this pull request Jun 2, 2017

Revert "Functional tests for the Getting Started page (elastic#11850)"

b382353

This reverts commit 099178a.

ycombinator mentioned this pull request Jun 2, 2017

Revert the Getting Started page #12154

Merged

ycombinator added a commit to ycombinator/kibana that referenced this pull request Jun 3, 2017

Revert "Functional tests for the Getting Started page (elastic#11850)"

4aee053

This reverts commit 099178a.

chrisronline mentioned this pull request May 17, 2017

Avoid duplication configuration within functional tests #11865

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functional tests for the Getting Started page #11850

Functional tests for the Getting Started page #11850

ycombinator commented May 17, 2017 •

edited

chrisronline May 17, 2017

chrisronline May 17, 2017

ycombinator May 17, 2017

ycombinator May 17, 2017

chrisronline May 17, 2017

ycombinator May 17, 2017

ycombinator May 17, 2017 •

edited

chrisronline May 17, 2017

ycombinator May 18, 2017

spalger May 17, 2017

spalger May 17, 2017

ycombinator May 18, 2017

spalger May 18, 2017

ycombinator May 18, 2017

ycombinator May 18, 2017

ycombinator May 18, 2017

ycombinator commented May 19, 2017

chrisronline left a comment

chrisronline May 19, 2017

ycombinator May 19, 2017

ycombinator commented May 22, 2017

spalger left a comment

ycombinator commented May 22, 2017 •

edited

ycombinator commented May 23, 2017


		const PageObjects = getPageObjects(['common', 'gettingStarted']);

		describe('Getting Started page', () => {

Functional tests for the Getting Started page #11850

Functional tests for the Getting Started page #11850

Conversation

ycombinator commented May 17, 2017 • edited

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

ycombinator May 17, 2017 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ycombinator commented May 19, 2017

chrisronline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ycombinator commented May 22, 2017

spalger left a comment

Choose a reason for hiding this comment

ycombinator commented May 22, 2017 • edited

ycombinator commented May 23, 2017

ycombinator commented May 17, 2017 •

edited

ycombinator May 17, 2017 •

edited

ycombinator commented May 22, 2017 •

edited