-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Support Cloud Kibana UI testing #8880
Conversation
This is in the CONTRIBUTING doc as an example for testing against Cloud;
|
To test this PR:
Do a |
}; | ||
|
||
exports.kibanaServer = { | ||
username: env.SHIELD_KIBANA_SERVER || 'kibana', | ||
password: env.SHIELD_KIBANA_SERVER_PASS || 'notsecure' | ||
username: env.TEST_KIBANA_SERVER_USER || 'elastic', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this default to kibana?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, fixed.
if (kibanaApp === 'login') { | ||
self.debug('Found loginPage username = ' | ||
+ config.servers.kibana.username | ||
+ 'password = ' + config.servers.kibana.password); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove password logging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I removed the password logging.
@@ -47,7 +47,7 @@ export default class HeaderPage { | |||
} | |||
|
|||
isTimepickerOpen() { | |||
return this.remote.setFindTimeout(defaultFindTimeout) | |||
return this.remote.setFindTimeout(2000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. When the timepicker is open we find it right away. When it's not open we don't want to wait the 10 second defaultFindTimeout.
This merge is targeting 5.0, is the plan to forward port it to master? |
@@ -20,6 +20,11 @@ bdd.describe('shared links', function describeIndexTests() { | |||
|
|||
bdd.before(function () { | |||
baseUrl = PageObjects.common.getHostPort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thoughts on replacing this with a common.getHost method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting that instead of stripping port :80
or :443
from baseUrl, that it could test to see if that was the port and if so, baseUrl = common.getHost()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or that common.getHostPort strip the port?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of using the getHostPort method and replacing the ports, adding another method called getHost() that returns without ports. my thoughts are mostly around naming, it's slightly confusing to use a method with get ports and then remove them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I'd still have to use .getHostPort()
to know if I need the .getHost()
or not.
Or I could use some other method to get the kibana port first from the config, and then either .getHostPort()
or .getHost()
.
So either this;
baseUrl = PageObjects.common.getHostPort();
if (baseUrl.includes(':80') || baseUrl.includes(':443') {
baseUrl = PageObjects.common.getHost();
}
or
if (config.kibana.port === '443' || config.kibana.port === '80') {
baseUrl = PageObjects.common.getHost();
} else {
baseUrl = PageObjects.common.getHostPort();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or, set the port to null or an empty string. Maybe it wouldn't be appended then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay I understand now, this lgtm.
@jbudz yes, I created this PR on the 5.0 branch because that's what's running on Elastic Cloud and we've already made changes in the tests in master which are not compatible with 5.0. But I will forward-port it to master. |
Backports PR #8880 **Commit 1:** Support Cloud Kibana testing * Original sha: c877b0d * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T19:16:31Z **Commit 2:** Merge branch '5.0' into supportCloudTesting * Original sha: 0c2815b * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T20:53:48Z **Commit 3:** Add xpack file * Original sha: d095dac * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-29T00:02:18Z **Commit 4:** Use kibana for kibana server user, Don't log password * Original sha: 9f63224 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-31T17:09:57Z
Backports PR #8880 **Commit 1:** Support Cloud Kibana testing * Original sha: c877b0d * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T19:16:31Z **Commit 2:** Merge branch '5.0' into supportCloudTesting * Original sha: 0c2815b * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T20:53:48Z **Commit 3:** Add xpack file * Original sha: d095dac * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-29T00:02:18Z **Commit 4:** Use kibana for kibana server user, Don't log password * Original sha: 9f63224 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-31T17:09:57Z
Backports PR #8915 **Commit 1:** Support Cloud Kibana UI testing master * Original sha: 98ba2ce * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T19:16:31Z **Commit 2:** Add xpack file * Original sha: adc27ba * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-29T00:02:18Z **Commit 3:** cherry-pick 9f63224 * Original sha: f4549ee * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-31T17:09:57Z
Backports PR #8915 **Commit 1:** Support Cloud Kibana UI testing master * Original sha: 98ba2ce * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T19:16:31Z **Commit 2:** Add xpack file * Original sha: adc27ba * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-29T00:02:18Z **Commit 3:** cherry-pick 9f63224 * Original sha: f4549ee * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-31T17:09:57Z
* Support Cloud Kibana UI testing master * Add xpack file * cherry-pick 9f63224
Backports PR elastic#8915 **Commit 1:** Support Cloud Kibana UI testing master * Original sha: 98ba2ce * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-28T19:16:31Z **Commit 2:** Add xpack file * Original sha: adc27ba * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-29T00:02:18Z **Commit 3:** cherry-pick 9f63224 * Original sha: f4549ee * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-10-31T17:09:57Z Former-commit-id: 9eb4f5b
Support Cloud Kibana testing; Fixes #8861
I moved the
run:devChromeDriver
grunt task from thetest:ui:server
task to thetest:ui:runner
task since then you only need to runtest:ui:runner
against some already running Elasticsearch and Kibana servers.If anyone wants to run this against my Cloud 5.0 instance I can give you the configuration settings.