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

SO Tagging: fix flaky test and re-enable it #82930

Merged
merged 11 commits into from
Nov 18, 2020
Merged

SO Tagging: fix flaky test and re-enable it #82930

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d216b9e
fix flaky test and re-enable it
pgayvallet Nov 9, 2020
c2d1238
wait for table to load before to perform operations
pgayvallet Nov 12, 2020
374814f
Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…
pgayvallet Nov 12, 2020
db86c3b
move everything out of ciGroup2 for flaky test runner
pgayvallet Nov 12, 2020
46da2af
add debug block for flaky runner
pgayvallet Nov 13, 2020
2909b19
use correct vis name
pgayvallet Nov 13, 2020
d37a5b7
Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…
pgayvallet Nov 16, 2020
c77f036
remove test sync
pgayvallet Nov 16, 2020
5e9a93c
Revert "move everything out of ciGroup2 for flaky test runner"
pgayvallet Nov 16, 2020
5861e50
Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…
pgayvallet Nov 18, 2020
61980e9
Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…
pgayvallet Nov 18, 2020
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
14 changes: 14 additions & 0 deletions test/functional/services/listing_table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ export function ListingTableProvider({ getService, getPageObjects }: FtrProvider
return visualizationNames;
}

public async waitUntilTableIsLoaded() {
return retry.try(async () => {
const isLoaded = await find.existsByDisplayedByCssSelector(
'[data-test-subj="itemsInMemTable"]:not(.euiBasicTable-loading)'
);

if (isLoaded) {
return true;
} else {
throw new Error('Waiting');
}
});
}

/**
* Navigates through all pages on Landing page and returns array of items names
*/
Expand Down
13 changes: 12 additions & 1 deletion x-pack/test/saved_object_tagging/functional/tests/dashboard_integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
const listingTable = getService('listingTable');
const testSubjects = getService('testSubjects');
const find = getService('find');
const PageObjects = getPageObjects(['dashboard', 'tagManagement', 'common']);
const PageObjects = getPageObjects(['dashboard', 'tagManagement', 'common', 'header']);

/**
* Select tags in the searchbar's tag filter.
Expand All @@ -31,6 +31,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
// click elsewhere to close the filter dropdown
const searchFilter = await find.byCssSelector('main .euiFieldSearch');
await searchFilter.click();
// wait until the table refreshes
await listingTable.waitUntilTableIsLoaded();
};

describe('dashboard integration', () => {
Expand All @@ -47,6 +49,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
beforeEach(async () => {
await PageObjects.common.navigateToApp('dashboard');
await PageObjects.dashboard.gotoDashboardLandingPage();
await listingTable.waitUntilTableIsLoaded();
});

it('allows to manually type tag filter query', async () => {
Expand Down Expand Up @@ -96,6 +99,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

await PageObjects.dashboard.gotoDashboardLandingPage();
await listingTable.waitUntilTableIsLoaded();

await selectFilterTags('tag-1');
const itemNames = await listingTable.getAllItemsNames();
expect(itemNames).to.contain('my-new-dashboard');
Expand Down Expand Up @@ -128,8 +133,11 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
expect(await tagModal.isOpened()).to.be(false);

await PageObjects.dashboard.clickSave();
await PageObjects.common.waitForSaveModalToClose();

await PageObjects.dashboard.gotoDashboardLandingPage();
await listingTable.waitUntilTableIsLoaded();

await selectFilterTags('my-new-tag');
const itemNames = await listingTable.getAllItemsNames();
expect(itemNames).to.contain('dashboard-with-new-tag');
Expand All @@ -140,6 +148,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
beforeEach(async () => {
await PageObjects.common.navigateToApp('dashboard');
await PageObjects.dashboard.gotoDashboardLandingPage();
await listingTable.waitUntilTableIsLoaded();
});

it('allows to select tags for an existing dashboard', async () => {
Expand All @@ -152,6 +161,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

await PageObjects.dashboard.gotoDashboardLandingPage();
await listingTable.waitUntilTableIsLoaded();

await selectFilterTags('tag-3');
const itemNames = await listingTable.getAllItemsNames();
expect(itemNames).to.contain('dashboard 4 with real data (tag-1)');
Expand Down
19 changes: 16 additions & 3 deletions x-pack/test/saved_object_tagging/functional/tests/visualize_integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
const listingTable = getService('listingTable');
const testSubjects = getService('testSubjects');
const find = getService('find');
const PageObjects = getPageObjects(['visualize', 'tagManagement', 'visEditor']);
const PageObjects = getPageObjects(['visualize', 'tagManagement', 'visEditor', 'common']);

/**
* Select tags in the searchbar's tag filter.
Expand All @@ -31,6 +31,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
// click elsewhere to close the filter dropdown
const searchFilter = await find.byCssSelector('main .euiFieldSearch');
await searchFilter.click();
// wait until the table refreshes
await listingTable.waitUntilTableIsLoaded();
};

const selectSavedObjectTags = async (...tagNames: string[]) => {
Expand All @@ -56,6 +58,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
describe('listing', () => {
beforeEach(async () => {
await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();
});

it('allows to manually type tag filter query', async () => {
Expand Down Expand Up @@ -83,7 +86,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

describe('creating', () => {
it.skip('allows to assign tags to the new visualization', async () => {
it('allows to assign tags to the new visualization', async () => {
await PageObjects.visualize.navigateToNewVisualization();

await PageObjects.visualize.clickMarkdownWidget();
Expand All @@ -95,14 +98,17 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
await selectSavedObjectTags('tag-1');

await testSubjects.click('confirmSaveSavedObjectButton');
await PageObjects.common.waitForSaveModalToClose();

await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();

await selectFilterTags('tag-1');
const itemNames = await listingTable.getAllItemsNames();
expect(itemNames).to.contain('My new markdown viz');
});

it('allows to assign tags to the new visualization', async () => {
it('allows to create a tag from the tag selector', async () => {
const { tagModal } = PageObjects.tagManagement;

await PageObjects.visualize.navigateToNewVisualization();
Expand Down Expand Up @@ -133,7 +139,10 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
expect(await tagModal.isOpened()).to.be(false);

await testSubjects.click('confirmSaveSavedObjectButton');
await PageObjects.common.waitForSaveModalToClose();
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need it?


await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();

await selectFilterTags('my-new-tag');
const itemNames = await listingTable.getAllItemsNames();
Expand All @@ -144,6 +153,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
describe('editing', () => {
beforeEach(async () => {
await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();
});

it('allows to assign tags to an existing visualization', async () => {
Expand All @@ -153,7 +163,10 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
await selectSavedObjectTags('tag-2');

await testSubjects.click('confirmSaveSavedObjectButton');
await PageObjects.common.waitForSaveModalToClose();

await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();
Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't understand whether we can get rid of this instruction in the test suit. As suggested in #82930 (comment)
IMO, It's okay in the preparatory phase but hard to debug in the test suite, so people start adding them all over the places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were some rare race conditions where the elements we are manipulating in the next instructions of the test were already removed from the DOM, because we retrieved them before the table was loaded (which cause a rerender, causing some existing dom elements to be removed from the document). This additional check is meant to fix the race by waiting for the next refresh


await selectFilterTags('tag-2');
const itemNames = await listingTable.getAllItemsNames();
Expand Down