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

[Discover] Unskip doc link functional test #66884

Merged
merged 6 commits into from
May 19, 2020
Merged

[Discover] Unskip doc link functional test #66884

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9896d89
Unksip test
kertal May 18, 2020
0a673db
Improve test
kertal May 18, 2020
838c2ca
Merge branch 'master' into kertal-pr-2020-05-18-discover_dov_navigati…
elasticmachine May 18, 2020
989b586
Merge branch 'master' into kertal-pr-2020-05-18-discover_dov_navigati…
elasticmachine May 18, 2020
c3f69f3
Improve test code by using retry.try
kertal May 18, 2020
11b093e
Merge branch 'kertal-pr-2020-05-18-discover_dov_navigation_functional…
kertal May 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
21 changes: 5 additions & 16 deletions test/functional/apps/discover/_doc_navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,39 +19,28 @@

import expect from '@kbn/expect';

const TEST_COLUMN_NAMES = ['@message'];
const TEST_FILTER_COLUMN_NAMES = [
['extension', 'jpg'],
['geo.src', 'IN'],
];

Copy link
Member Author

Choose a reason for hiding this comment

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

It's no necessary for the test to add columns and filters

export default function({ getService, getPageObjects }) {
const docTable = getService('docTable');
const testSubjects = getService('testSubjects');
const PageObjects = getPageObjects(['common', 'discover', 'timePicker']);
const esArchiver = getService('esArchiver');

// FLAKY: https://github.com/elastic/kibana/issues/62281
describe.skip('doc link in discover', function contextSize() {
describe('doc link in discover', function contextSize() {
before(async function() {
await esArchiver.loadIfNeeded('logstash_functional');
await PageObjects.common.navigateToApp('discover');
await PageObjects.timePicker.setDefaultAbsoluteRange();
await PageObjects.discover.waitForDocTableLoadingComplete();
for (const columnName of TEST_COLUMN_NAMES) {
await PageObjects.discover.clickFieldListItemAdd(columnName);
}
for (const [columnName, value] of TEST_FILTER_COLUMN_NAMES) {
await PageObjects.discover.clickFieldListItem(columnName);
await PageObjects.discover.clickFieldListPlusFilter(columnName, value);
}
});

it('should open the doc view of the selected document', async function() {
// navigate to the doc view
await docTable.clickRowToggle({ rowIndex: 0 });
await (await docTable.getRowActions({ rowIndex: 0 }))[1].click();
Copy link
Member Author

@kertal kertal May 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Here is the point where some failures happened. Maybe the navigation wasn't rendered / to late?

TypeError: Cannot read property 'click' of undefined
    at Context.<anonymous> (test/functional/apps/discover/_doc_navigation.js:53:64)
    at process._tickCallback (internal/process/next_tick.js:68:7)

but the screens I've got were not usefull, since they were scrolled down. In a future failure there should be a useful screen, also I thought that maybe a bit of sleep could prevent a possible race condition between opening the document detail and clicking the row action button.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a bit of sleep could prevent a possible race condition between opening the document detail and clicking the row action button

Sounds like what you want is a retry.waitFor(() => checkIfRowOpenedSomehow()). Putting an arbitrary sleep in here doesn't really prevent anything since the timing on CI is wildly unpredictable. Only way to add safeguards is with retries that are based on specific checks.

Copy link
Contributor

@spalger spalger May 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, even better:

// click the open action
await retry.try(async () => {
  const rowActions = await docTable.getRowActions({ rowIndex: 0 });
  if (!rowActions.length) {
    throw new Error('row actions empty, trying again')
  }
  
  await rowActions[1].click()
})

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed and adapted the code for this case, recent flaky run also just finished successfully (https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/465).
I'd love to have a kind of alert when this test would fail again

await PageObjects.common.sleep(500);
const rowActions = await docTable.getRowActions({ rowIndex: 0 });
expect(rowActions.length).to.be(2);

await rowActions[1].click();
const hasDocHit = await testSubjects.exists('doc-hit');
expect(hasDocHit).to.be(true);
});
Expand Down