Skip to content

Commit

Permalink
Fix possible flakiness of saveVisualization action (elastic#22356)
Browse files Browse the repository at this point in the history
* Updated saveVisualization to wait for toast instead of global indicator

* Fix RBAC tests.

The saveVisualization now fails directly if the visualization is not correctly saved.

* Change editor to use toast for errors

* Change saveVisualization method to new saveVisualizationExpectSuccess

For RBAC tests used also saveVisualizationExpectFail

* Fix wrong exists method call

* Fix missing exist function
  • Loading branch information
markov00 committed Aug 30, 2018
1 parent 2a190be commit d0a039c
Show file tree
Hide file tree
Showing 20 changed files with 50 additions and 38 deletions.
14 changes: 8 additions & 6 deletions src/core_plugins/kibana/public/visualize/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import 'ui/collapsible_sidebar';
import 'ui/query_bar';
import chrome from 'ui/chrome';
import angular from 'angular';
import { Notifier, toastNotifications } from 'ui/notify';
import { toastNotifications } from 'ui/notify';
import { VisTypesRegistryProvider } from 'ui/registry/vis_types';
import { DocTitleProvider } from 'ui/doc_title';
import { FilterBarQueryFilterProvider } from 'ui/filter_bar/query_filter';
Expand Down Expand Up @@ -118,10 +118,6 @@ function VisEditor(
const queryFilter = Private(FilterBarQueryFilterProvider);
const getUnhashableStates = Private(getUnhashableStatesProvider);

const notify = new Notifier({
location: 'Visualization Editor'
});

// Retrieve the resolved SavedVis instance.
const savedVis = $route.current.locals.savedVis;
// vis is instance of src/ui/public/vis/vis.js.
Expand Down Expand Up @@ -387,7 +383,13 @@ function VisEditor(
kbnUrl.change(`${VisualizeConstants.EDIT_PATH}/{{id}}`, { id: savedVis.id });
}
}
}, notify.error);
}, (err) => {
toastNotifications.addDanger({
title: `Error on saving '${savedVis.title}'`,
text: err.message,
'data-test-subj': 'saveVisualizationError',
});
});
};

$scope.unlink = function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default function ({ getService, getPageObjects }) {
await dashboardAddPanel.clickAddNewEmbeddableLink();
await PageObjects.visualize.clickAreaChart();
await PageObjects.visualize.clickNewSearch();
await PageObjects.visualize.saveVisualization('visualization from add new link');
await PageObjects.visualize.saveVisualizationExpectSuccess('visualization from add new link');

return retry.try(async () => {
const panelCount = await PageObjects.dashboard.getPanelCount();
Expand Down
4 changes: 2 additions & 2 deletions test/functional/apps/dashboard/_dashboard_filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export default function ({ getService, getPageObjects }) {
await renderable.waitForRender();
await dashboardExpect.pieSliceCount(3);

await PageObjects.visualize.saveVisualization('Rendering Test: animal sounds pie');
await PageObjects.visualize.saveVisualizationExpectSuccess('Rendering Test: animal sounds pie');
await PageObjects.header.clickDashboard();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.dashboard.waitForRenderComplete();
Expand All @@ -260,7 +260,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.header.waitUntilLoadingHasFinished();
await dashboardExpect.pieSliceCount(1);

await PageObjects.visualize.saveVisualization('animal sounds pie');
await PageObjects.visualize.saveVisualizationExpectSuccess('animal sounds pie');
await PageObjects.header.clickDashboard();

await dashboardExpect.pieSliceCount(1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/dashboard/_dashboard_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();

await PageObjects.visualize.saveVisualization('Visualization TileMap');
await PageObjects.visualize.saveVisualizationExpectSuccess('Visualization TileMap');

await PageObjects.header.clickDashboard();

Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/dashboard/_view_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default function ({ getService, getPageObjects }) {
await dashboardAddPanel.clickAddNewEmbeddableLink();
await PageObjects.visualize.clickAreaChart();
await PageObjects.visualize.clickNewSearch();
await PageObjects.visualize.saveVisualization('new viz panel');
await PageObjects.visualize.saveVisualizationExpectSuccess('new viz panel');

await PageObjects.dashboard.clickCancelOutOfEditMode();

Expand Down
6 changes: 3 additions & 3 deletions test/functional/apps/visualize/_area_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default function ({ getService, getPageObjects }) {

it('should save and load with special characters', async function () {
const vizNamewithSpecialChars = vizName1 + '/?&=%';
await PageObjects.visualize.saveVisualization(vizNamewithSpecialChars);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizNamewithSpecialChars);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizNamewithSpecialChars);
Expand All @@ -67,14 +67,14 @@ export default function ({ getService, getPageObjects }) {

it('should save and load with non-ascii characters', async function () {
const vizNamewithSpecialChars = `${vizName1} with Umlaut ä`;
await PageObjects.visualize.saveVisualization(vizNamewithSpecialChars);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizNamewithSpecialChars);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Saved viz page title with umlaut is ${pageTitle}`);
expect(pageTitle).to.contain(vizNamewithSpecialChars);
});

it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Saved viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_data_table.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should be able to save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_data_table_nontimeindex.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should be able to save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
4 changes: 2 additions & 2 deletions test/functional/apps/visualize/_heatmap_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand All @@ -58,7 +58,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_line_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should be able to save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_linked_saved_searches.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should not break when saving after unlinking', async () => {
await PageObjects.visualize.saveVisualization('Unlinked before saved');
await PageObjects.visualize.saveVisualizationExpectSuccess('Unlinked before saved');
await PageObjects.header.waitUntilLoadingHasFinished();
await retry.waitFor('wait for count to equal 1,293', async () => {
const data = await PageObjects.visualize.getTableVisData();
Expand Down
4 changes: 2 additions & 2 deletions test/functional/apps/visualize/_pie_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down Expand Up @@ -139,7 +139,7 @@ export default function ({ getService, getPageObjects }) {
});

it('should correctly save disabled agg', async () => {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_tag_cloud.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export default function ({ getService, getPageObjects }) {


it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_tile_map.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export default function ({ getService, getPageObjects }) {
const mapBounds = await PageObjects.visualize.getMapBounds();
await PageObjects.visualize.closeInspector();

await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);

const afterSaveMapBounds = await PageObjects.visualize.getMapBounds();

Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/visualize/_vertical_bar_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default function ({ getService, getPageObjects }) {
before(initBarChart);

it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function ({ getService, getPageObjects }) {
before(initBarChart);

it('should save and load', async function () {
await PageObjects.visualize.saveVisualization(vizName1);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
const pageTitle = await PageObjects.common.getBreadcrumbPageTitle();
log.debug(`Save viz page title is ${pageTitle}`);
expect(pageTitle).to.contain(vizName1);
Expand Down
22 changes: 17 additions & 5 deletions test/functional/page_objects/visualize_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
*/

import { VisualizeConstants } from '../../../src/core_plugins/kibana/public/visualize/visualize_constants';
import Keys from 'leadfoot/keys';
import Bluebird from 'bluebird';
import expect from 'expect.js';
import Keys from 'leadfoot/keys';

export function VisualizePageProvider({ getService, getPageObjects }) {
const remote = getService('remote');
Expand Down Expand Up @@ -712,13 +713,24 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
async saveVisualization(vizName, { saveAsNew = false } = {}) {
await this.ensureSavePanelOpen();
await testSubjects.setValue('visTitleInput', vizName);
log.debug('click submit button');
await testSubjects.click('saveVisualizationButton');
if (saveAsNew) {
log.debug('Check save as new visualization');
await testSubjects.click('saveAsNewCheckbox');
}
await PageObjects.header.waitUntilLoadingHasFinished();
return await testSubjects.exists('saveVisualizationSuccess');
log.debug('Click Save Visualization button');
await testSubjects.click('saveVisualizationButton');
}

async saveVisualizationExpectSuccess(vizName, { saveAsNew = false } = {}) {
await this.saveVisualization(vizName, { saveAsNew });
const successToast = await testSubjects.exists('saveVisualizationSuccess', defaultFindTimeout);
expect(successToast).to.be(true);
}

async saveVisualizationExpectFail(vizName, { saveAsNew = false } = {}) {
await this.saveVisualization(vizName, { saveAsNew });
const errorToast = await testSubjects.exists('saveVisualizationError', defaultFindTimeout);
expect(errorToast).to.be(true);
}

async clickLoadSavedVisButton() {
Expand Down
4 changes: 2 additions & 2 deletions test/functional/services/dashboard/visualizations.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function DashboardVisualizationProvider({ getService, getPageObjects }) {
await dashboardAddPanel.ensureAddPanelIsShowing();
await dashboardAddPanel.clickAddNewEmbeddableLink();
await PageObjects.visualize.clickVisualBuilder();
await PageObjects.visualize.saveVisualization(name);
await PageObjects.visualize.saveVisualizationExpectSuccess(name);
}

async createSavedSearch({ name, query, fields }) {
Expand Down Expand Up @@ -84,7 +84,7 @@ export function DashboardVisualizationProvider({ getService, getPageObjects }) {
await PageObjects.visualize.clickMarkdownWidget();
await PageObjects.visualize.setMarkdownTxt(markdown);
await PageObjects.visualize.clickGo();
await PageObjects.visualize.saveVisualization(name);
await PageObjects.visualize.saveVisualizationExpectSuccess(name);
}
};
}
6 changes: 2 additions & 4 deletions x-pack/test/functional/apps/security/rbac_phase1.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.visualize.waitForVisualization();
const success = await PageObjects.visualize.saveVisualization(vizName1);
expect(success).to.be(true);
await PageObjects.visualize.saveVisualizationExpectSuccess(vizName1);
await PageObjects.security.logout();

});
Expand All @@ -110,8 +109,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.visualize.waitForVisualization();
const success = await PageObjects.visualize.saveVisualization(vizName1);
expect(success).to.be(false);
await PageObjects.visualize.saveVisualizationExpectFail(vizName1);
await PageObjects.security.logout();

});
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/reporting/functional/reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.visualize.clickBucket('X-Axis');
await PageObjects.visualize.selectAggregation('Date Histogram');
await PageObjects.visualize.clickGo();
await PageObjects.visualize.saveVisualization('my viz');
await PageObjects.visualize.saveVisualizationExpectSuccess('my viz');
await expectEnabledGenerateReportButton();
});

Expand Down

0 comments on commit d0a039c

Please sign in to comment.