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.

Sign up for GitHub

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

remove inline awaits #13043

Merged
merged 5 commits into from
Jul 25, 2017
Merged

remove inline awaits #13043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5bbab88
remove inline awaits
stacey-gammon Jul 21, 2017
11e383d
testSubjects.find now always returns a promise, so we need to make su…
stacey-gammon Jul 21, 2017
f6ee338
more find conversions
stacey-gammon Jul 21, 2017
72390a8
need to pass property name
stacey-gammon Jul 24, 2017
5aade92
Make sure async functions are awaited on.
stacey-gammon Jul 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions test/functional/apps/dashboard/_view_edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export default function ({ getService, getPageObjects }) {

it('when the query is edited and applied', async function () {
const originalQuery = await PageObjects.dashboard.getQuery();
await PageObjects.dashboard.appendQuery('extra stuff');
await PageObjects.dashboard.setQuery(`${originalQuery} and extra stuff`);
await PageObjects.dashboard.clickFilterButton();

await PageObjects.dashboard.clickCancelOutOfEditMode();
Expand Down Expand Up @@ -184,8 +184,7 @@ export default function ({ getService, getPageObjects }) {
await originalFilters[0].click();
await originalFilters[0].click();

const removeFilterButton = await testSubjects.find('removeFilter-memory');
await removeFilterButton.click();
await testSubjects.click('removeFilter-memory');

const noFilters = await PageObjects.dashboard.getFilters(1000);
expect(noFilters.length).to.equal(0);
Expand Down Expand Up @@ -288,7 +287,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.dashboard.gotoDashboardEditMode(dashboardName);

const originalQuery = await PageObjects.dashboard.getQuery();
await PageObjects.dashboard.appendQuery('extra stuff');
await PageObjects.dashboard.setQuery(`${originalQuery} extra stuff`);

await PageObjects.dashboard.clickCancelOutOfEditMode();

Expand Down
15 changes: 6 additions & 9 deletions test/functional/apps/management/_creation_form_changes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@ export default function ({ getService, getPageObjects }) {
});
});

it('should enable creation after selecting time field', function () {
it('should enable creation after selecting time field', async function () {
// select a time field and check that Create button is enabled
return PageObjects.settings.selectTimeFieldOption('@timestamp')
.then(function () {
return PageObjects.settings.getCreateButton().isEnabled()
.then(function (enabled) {
screenshots.take('Settings-indices-enable-creation');
expect(enabled).to.be.ok();
});
});
await PageObjects.settings.selectTimeFieldOption('@timestamp');
const createButton = await PageObjects.settings.getCreateButton();
const enabled = await createButton.isEnabled();
screenshots.take('Settings-indices-enable-creation');
expect(enabled).to.be.ok();
});
});
}
11 changes: 5 additions & 6 deletions test/functional/apps/management/_index_pattern_create_delete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ export default function ({ getService, getPageObjects }) {
.then(id => indexPatternId = id);
});

it('should have index pattern in page header', function () {
return PageObjects.settings.getIndexPageHeading().getVisibleText()
.then(function (patternName) {
screenshots.take('Settings-indices-new-index-pattern');
expect(patternName).to.be('logstash-*');
});
it('should have index pattern in page header', async function () {
const indexPageHeading = await PageObjects.settings.getIndexPageHeading();
const patternName = await indexPageHeading.getVisibleText();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not super well versed in async/await, but I thought it was basically just like wrapping the call in a then handler. If I'm right about that, couldn't this be written like so?

const patternName = await PageObjects.settings.getIndexPageHeading().getVisibleText();

Copy link
Contributor

Choose a reason for hiding this comment

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

Like return, await affects the entire expression, not just a function invocation. So await doesn't really "kick in" until it finds a newline, a semi-colon, the closing of a block, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we don't want to use multiple functions on the same line as an await because it's confusing (there was a little discussion in the kibana slack room and I filed #13041). Does the await apply to the first or second? Sans parens, it applies to the second. This causes extra confusion with tests if a function returns the webdriver API - it can be used as a promise, or chained together with the call that should go after the promise.

testSubjects.find changed so that it's not returning the webdriver API, it's returning a legit promise, which means we can't get away with this subtle behavior anymore. Now, the await has to be used for the first function, not the second. So you could write it like this:

const patternName = (await PageObjects.settings.getIndexPageHeading()).getVisibleText();

But to avoid all this confusion, we just decided to not use parens and not use multiple functions on the same line when an await is required.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, thanks for the education! 👯‍♂️

screenshots.take('Settings-indices-new-index-pattern');
expect(patternName).to.be('logstash-*');
});

it('should have index pattern in url', function url() {
Expand Down
29 changes: 13 additions & 16 deletions test/functional/apps/management/_initial_state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,25 @@ export default function ({ getService, getPageObjects }) {
});
});

it('should contain default index pattern', function () {
it('should contain default index pattern', async function () {
const defaultPattern = 'logstash-*';

return PageObjects.settings.getIndexPatternField().getProperty('value')
.then(function (pattern) {
expect(pattern).to.be(defaultPattern);
});
const indexPatternField = await PageObjects.settings.getIndexPatternField();
const pattern = await indexPatternField.getProperty('value');
expect(pattern).to.be(defaultPattern);
});

it('should not select the time field', function () {
return PageObjects.settings.getTimeFieldNameField().isSelected()
.then(function (timeFieldIsSelected) {
log.debug('timeField isSelected = ' + timeFieldIsSelected);
expect(timeFieldIsSelected).to.not.be.ok();
});
it('should not select the time field', async function () {
const timeFieldNameField = await PageObjects.settings.getTimeFieldNameField();
const timeFieldIsSelected = await timeFieldNameField.isSelected();
log.debug('timeField isSelected = ' + timeFieldIsSelected);
expect(timeFieldIsSelected).to.not.be.ok();
});

it('should not enable creation', function () {
return PageObjects.settings.getCreateIndexPatternButton().isEnabled()
.then(function (enabled) {
expect(enabled).to.not.be.ok();
});
it('should not enable creation', async function () {
const createIndexPatternButton = await PageObjects.settings.getCreateIndexPatternButton();
const enabled = await createIndexPatternButton.isEnabled();
expect(enabled).to.not.be.ok();
});
});
}
13 changes: 5 additions & 8 deletions test/functional/apps/status_page/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.common.navigateToApp('status_page');
});

it('should show the kibana plugin as ready', function () {
return retry.tryForTime(6000, function () {
return testSubjects.find('statusBreakdown')
.getVisibleText()
.then(function (text) {
screenshots.take('Status');
expect(text.indexOf('plugin:kibana')).to.be.above(-1);
});
it('should show the kibana plugin as ready', async function () {
await retry.tryForTime(6000, async () => {
const text = await testSubjects.getVisibleText('statusBreakdown');
screenshots.take('Status');
expect(text.indexOf('plugin:kibana')).to.be.above(-1);
});
});
});
Expand Down
23 changes: 11 additions & 12 deletions test/functional/page_objects/context_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,27 @@ export function ContextPageProvider({ getService }) {
await this.waitUntilContextLoadingHasFinished();
}

getPredecessorCountPicker() {
return testSubjects.find('predecessorCountPicker');
async getPredecessorCountPicker() {
return await testSubjects.find('predecessorCountPicker');
}

getSuccessorCountPicker() {
return testSubjects.find('successorCountPicker');
async getSuccessorCountPicker() {
return await testSubjects.find('successorCountPicker');
}

getPredecessorLoadMoreButton() {
return testSubjects.find('predecessorLoadMoreButton');
async getPredecessorLoadMoreButton() {
return await testSubjects.find('predecessorLoadMoreButton');
}

getSuccessorLoadMoreButton() {
return testSubjects.find('predecessorLoadMoreButton');
async getSuccessorLoadMoreButton() {
return await testSubjects.find('predecessorLoadMoreButton');
}

waitUntilContextLoadingHasFinished() {
return retry.try(async () => {
if (
!(await this.getSuccessorLoadMoreButton().isEnabled())
|| !(await this.getPredecessorLoadMoreButton().isEnabled())
) {
const successorLoadMoreButton = await this.getSuccessorLoadMoreButton();
const predecessorLoadMoreButton = await this.getPredecessorLoadMoreButton();
if (!successorLoadMoreButton.isEnabled() || !predecessorLoadMoreButton.isEnabled()) {
throw new Error('loading context rows');
}
});
Expand Down
27 changes: 12 additions & 15 deletions test/functional/page_objects/dashboard_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
return await queryObject.getProperty('value');
}

appendQuery(query) {
log.debug('Appending query');
return retry.try(() => testSubjects.find('queryInput').type(query));
async setQuery(query) {
log.debug(`setQuery(${query})`);
return await testSubjects.setValue('queryInput', query);
}

clickFilterButton() {
async clickFilterButton() {
log.debug('Clicking filter button');
return testSubjects.click('querySubmitButton');
return await testSubjects.click('querySubmitButton');
}

async clickClone() {
Expand Down Expand Up @@ -327,18 +327,15 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
});
}

getPanelTitles() {
async getPanelTitles() {
log.debug('in getPanelTitles');
return testSubjects.findAll('dashboardPanelTitle')
.then(function (titleObjects) {

function getTitles(chart) {
return chart.getVisibleText();
}
const titleObjects = await testSubjects.findAll('dashboardPanelTitle');

const getTitlePromises = titleObjects.map(getTitles);
return Promise.all(getTitlePromises);
});
function getTitles(chart) {
return chart.getVisibleText();
}
const getTitlePromises = titleObjects.map(getTitles);
return Promise.all(getTitlePromises);
}

async getDashboardPanels() {
Expand Down
72 changes: 28 additions & 44 deletions test/functional/page_objects/discover_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
const log = getService('log');
const retry = getService('retry');
const testSubjects = getService('testSubjects');
const find = getService('find');
const PageObjects = getPageObjects(['header', 'common']);

const getRemote = () => (
Expand All @@ -21,9 +22,8 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
.findByCssSelector('button[aria-label=\'Search\']');
}

getTimespanText() {
return testSubjects.find('globalTimepickerRange')
.getVisibleText();
async getTimespanText() {
return await testSubjects.getVisibleText('globalTimepickerRange');
}

getChartTimespan() {
Expand Down Expand Up @@ -67,9 +67,8 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
return testSubjects.click('discoverOpenButton');
}

getCurrentQueryName() {
return testSubjects.find('discoverCurrentQuery')
.getVisibleText();
async getCurrentQueryName() {
return await testSubjects.getVisibleText('discoverCurrentQuery');
}

getBarChartData() {
Expand Down Expand Up @@ -126,32 +125,21 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
});
}

getChartInterval() {
return testSubjects.find('discoverIntervalSelect')
.getProperty('value')
.then(selectedValue => {
return getRemote()
.findByCssSelector('option[value="' + selectedValue + '"]')
.getVisibleText();
});
async getChartInterval() {
const selectedValue = await testSubjects.getProperty('discoverIntervalSelect', 'value');
const selectedOption = await find.byCssSelector('option[value="' + selectedValue + '"]');
return selectedOption.getVisibleText();
}

setChartInterval(interval) {
return getRemote()
.setFindTimeout(5000)
.findByCssSelector('option[label="' + interval + '"]')
.click()
.then(() => {
return PageObjects.header.waitUntilLoadingHasFinished();
});
async setChartInterval(interval) {
const optionElement = await find.byCssSelector('option[label="' + interval + '"]', 5000);
await optionElement.click();
return await PageObjects.header.waitUntilLoadingHasFinished();
}

getHitCount() {
return PageObjects.header.waitUntilLoadingHasFinished()
.then(() => {
return testSubjects.find('discoverQueryHits')
.getVisibleText();
});
async getHitCount() {
await PageObjects.header.waitUntilLoadingHasFinished();
return await testSubjects.getVisibleText('discoverQueryHits');
}

query(queryString) {
Expand Down Expand Up @@ -211,18 +199,16 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
return testSubjects.click('sharedSnapshotCopyButton');
}

getShareCaption() {
return testSubjects.find('shareUiTitle')
.getVisibleText();
async getShareCaption() {
return await testSubjects.getVisibleText('shareUiTitle');
}

getSharedUrl() {
return testSubjects.find('sharedSnapshotUrl')
.getProperty('value');
async getSharedUrl() {
return await testSubjects.getProperty('sharedSnapshotUrl', 'value');
}

toggleSidebarCollapse() {
return testSubjects.find('collapseSideBarButton').click();
async toggleSidebarCollapse() {
return await testSubjects.click('collapseSideBarButton');
}

getAllFieldNames() {
Expand All @@ -239,14 +225,12 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
.getProperty('clientWidth');
}

hasNoResults() {
return testSubjects.find('discoverNoResults')
.then(() => true)
.catch(() => false);
async hasNoResults() {
return await testSubjects.exists('discoverNoResults');
}

getNoResultsTimepicker() {
return testSubjects.find('discoverNoResultsTimefilter');
async getNoResultsTimepicker() {
return await testSubjects.find('discoverNoResultsTimefilter');
}

hasNoResultsTimepicker() {
Expand All @@ -256,8 +240,8 @@ export function DiscoverPageProvider({ getService, getPageObjects }) {
.catch(() => false);
}

clickFieldListItem(field) {
return testSubjects.click(`field-${field}`);
async clickFieldListItem(field) {
return await testSubjects.click(`field-${field}`);
}

async clickFieldListItemAdd(field) {
Expand Down
8 changes: 2 additions & 6 deletions test/functional/page_objects/header_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,11 @@ export function HeaderPageProvider({ getService, getPageObjects }) {
}

async getPrettyDuration() {
return await testSubjects.find('globalTimepickerRange').getVisibleText();
return await testSubjects.getVisibleText('globalTimepickerRange');
}

async isSharedTimefilterEnabled() {
const element = await remote
.setFindTimeout(defaultFindTimeout)
.findByCssSelector(`[shared-timefilter=true]`);

return !!element;
return await find.existsByCssSelector('[shared-timefilter=true]');
}
}

Expand Down
5 changes: 2 additions & 3 deletions test/functional/page_objects/point_series_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ export function PointSeriesPageProvider({ getService }) {
.click();
}

clickAddAxis() {
return testSubjects.find('visualizeAddYAxisButton')
.click();
async clickAddAxis() {
return await testSubjects.click('visualizeAddYAxisButton');
}

getValueAxesCount() {
Expand Down
Loading