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.

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

Getting flaky tests back in shape for reporting #46076

Merged
merged 20 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e8ccb8e
Rebasing from master, updating test utils and getting report pdf/png …
Jan 6, 2020
edb3b7a
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 6, 2020
b10f8a7
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 6, 2020
29c7e9b
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 7, 2020
b04d9e5
Removing legacy functions, packages and updating README/Licenses
Jan 13, 2020
0d118e1
Merge branch 'chore/reporting-fix-flaky-tests' of https://github.com/…
Jan 13, 2020
68cdaef
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 13, 2020
ee3d3f7
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 13, 2020
b9d08c7
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 14, 2020
841b790
Dropping duplicitive test
Jan 14, 2020
347a814
Merge branch 'chore/reporting-fix-flaky-tests' of https://github.com/…
Jan 14, 2020
c2d34a3
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 14, 2020
b24ab73
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 14, 2020
ac9dd96
Better URL check for lens reporting
Jan 14, 2020
cd1ce2b
Merge branch 'chore/reporting-fix-flaky-tests' of https://github.com/…
Jan 14, 2020
382be06
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 14, 2020
45984d9
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 15, 2020
e27c04f
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 15, 2020
8d826d2
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 15, 2020
7180be9
Merge branch 'master' into chore/reporting-fix-flaky-tests
elasticmachine Jan 15, 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
3 changes: 0 additions & 3 deletions src/dev/license_checker/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@ export const LICENSE_OVERRIDES = {
// TODO can be removed if the PR#9 is accepted on the source
'pause-stream@0.0.11': ['MIT'],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dep no longer used

// TODO can be removed once we upgrade past or equal pdf-image@2.0.1
'pdf-image@1.1.0': ['MIT'],

// TODO can be removed once we upgrade the use of walk dependency past or equal to v2.3.14
'walk@2.3.9': ['MIT'],
};
6 changes: 6 additions & 0 deletions test/functional/page_objects/common_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ export function CommonPageProvider({ getService, getPageObjects }: FtrProviderCo
await browser.pressKeys(browser.keys.ENTER);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is nice little helper for debugging tests when needed, left for future users

// Pause the browser at a certain place for debugging
// Not meant for usage in CI, only for dev-usage
async pause() {
return browser.pause();
}

/**
* Clicks cancel button on modal
* @param overlayWillStay pass in true if your test will show multiple modals in succession
Expand Down
8 changes: 8 additions & 0 deletions test/functional/services/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,14 @@ export async function BrowserProvider({ getService }: FtrProviderContext) {
return await driver.get(url);
}

/**
* Pauses the execution in the browser, similar to setting a breakpoint for debugging.
* @return {Promise<void>}
*/
public async pause() {
await driver.executeAsyncScript(`(async () => { debugger; return Promise.resolve(); })()`);
Copy link
Member

Choose a reason for hiding this comment

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

When/If this turns out to be useful, can you add a note in some developer docs about how to use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Great reminder, thanks!

}

/**
* Moves the remote environment’s mouse cursor to the specified point {x, y} which is
* offset to browser page top left corner.
Expand Down
6 changes: 4 additions & 2 deletions test/functional/services/dashboard/add_panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export function DashboardAddPanelProvider({ getService, getPageObjects }) {
async clickOpenAddPanel() {
log.debug('DashboardAddPanel.clickOpenAddPanel');
await testSubjects.click('dashboardAddPanelButton');
// Give some time for the animation to complete
await PageObjects.common.sleep(500);
}

async clickAddNewEmbeddableLink(type) {
Expand Down Expand Up @@ -96,8 +98,8 @@ export function DashboardAddPanelProvider({ getService, getPageObjects }) {
if (!isOpen) {
await retry.try(async () => {
await this.clickOpenAddPanel();
const isOpen = await this.isAddPanelOpen();
if (!isOpen) {
const isNowOpen = await this.isAddPanelOpen();
if (!isNowOpen) {
throw new Error('Add panel still not open, trying again.');
}
});
Expand Down
5 changes: 5 additions & 0 deletions test/functional/services/remote/webdriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { Browsers } from './browsers';

const throttleOption: string = process.env.TEST_THROTTLE_NETWORK as string;
const headlessBrowser: string = process.env.TEST_BROWSER_HEADLESS as string;
const remoteDebug: string = process.env.TEST_REMOTE_DEBUG as string;
const SECOND = 1000;
const MINUTE = 60 * SECOND;
const NO_QUEUE_COMMANDS = ['getLog', 'getStatus', 'newSession', 'quit'];
Expand Down Expand Up @@ -97,6 +98,10 @@ async function attemptToCreateCommand(
// See: https://chromium.googlesource.com/chromium/src/+/lkgr/headless/README.md
chromeOptions.push('headless', 'disable-gpu');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added this so you can remotely debug (view with devtools) the session as it's running headlessly

if (remoteDebug === '1') {
// Visit chrome://inspect in chrome to remotely view/debug
chromeOptions.push('headless', 'disable-gpu', 'remote-debugging-port=9222');
}
chromeCapabilities.set('goog:chromeOptions', {
w3c: false,
args: chromeOptions,
Expand Down
8 changes: 6 additions & 2 deletions test/functional/services/test_subjects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,14 @@ export function TestSubjectsProvider({ getService }: FtrProviderContext) {
});
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Underlying selenium method allows for a timeout, so I added it as an optional param here.

public async getAttribute(selector: string, attribute: string): Promise<string> {
public async getAttribute(
selector: string,
attribute: string,
timeout?: number
): Promise<string> {
return await retry.try(async () => {
log.debug(`TestSubjects.getAttribute(${selector}, ${attribute})`);
const element = await this.find(selector);
const element = await this.find(selector, timeout);
return await element.getAttribute(attribute);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,7 @@ describe('CSV Execute Job', function() {
});
});

// FLAKY: https://github.com/elastic/kibana/issues/43069
describe.skip('cancellation', function() {
describe('cancellation', function() {
const scrollId = getRandomScrollId();

beforeEach(function() {
Expand All @@ -618,10 +617,10 @@ describe('CSV Execute Job', function() {
cancellationToken
);

await delay(100);
await delay(250);
const callCount = callWithRequestStub.callCount;
cancellationToken.cancel();
await delay(100);
await delay(250);
expect(callWithRequestStub.callCount).to.be(callCount + 1); // last call is to clear the scroll
});

Expand Down
11 changes: 9 additions & 2 deletions x-pack/legacy/plugins/reporting/public/lib/download_report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@
import chrome from 'ui/chrome';
import { API_BASE_URL } from '../../common/constants';

export function downloadReport(jobId: string) {
export function getReportURL(jobId: string) {
const apiBaseUrl = chrome.addBasePath(API_BASE_URL);
const downloadLink = `${apiBaseUrl}/jobs/download/${jobId}`;
window.open(downloadLink);

return downloadLink;
}

export function downloadReport(jobId: string) {
const location = getReportURL(jobId);

window.open(location);
}
2 changes: 0 additions & 2 deletions x-pack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,6 @@
"mutation-observer": "^1.0.3",
"node-fetch": "^2.6.0",
"null-loader": "^3.0.0",
"pdf-image": "2.0.0",
"pdfjs-dist": "^2.0.943",
"pixelmatch": "^5.1.0",
"proxyquire": "1.8.0",
"react-docgen-typescript-loader": "^3.1.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,12 @@ interface Props {
}

export const DownloadButton = ({ getUrl, job }: Props) => {
const downloadReport = () => {
window.open(getUrl(job.id));
};

return (
<EuiButton
size="s"
data-test-subj="downloadCompletedReportButton"
onClick={() => {
downloadReport();
}}
href={getUrl(job.id)}
target="_blank"
>
<FormattedMessage
id="xpack.reporting.publicNotifier.downloadReportButtonLabel"
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/functional/apps/lens/lens_reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { FtrProviderContext } from '../../ftr_provider_context';
// eslint-disable-next-line import/no-default-export
export default function({ getService, getPageObjects }: FtrProviderContext) {
const PageObjects = getPageObjects(['common', 'dashboard', 'reporting']);
const find = getService('find');
const esArchiver = getService('esArchiver');
const listingTable = getService('listingTable');

Expand All @@ -28,8 +27,9 @@ export default function({ getService, getPageObjects }: FtrProviderContext) {
await listingTable.clickItemLink('dashboard', 'Lens reportz');
await PageObjects.reporting.openPdfReportingPanel();
await PageObjects.reporting.clickGenerateReportButton();
const url = await PageObjects.reporting.getReportURL(60000);

expect(await find.byButtonText('Download report', undefined, 60000)).to.be.ok();
expect(url).to.be.ok();
});
});
}
71 changes: 28 additions & 43 deletions x-pack/test/functional/page_objects/reporting_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,6 @@ export function ReportingPageProvider({ getService, getPageObjects }) {
await browser.setWindowSize(1600, 850);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer need theses now that we aren't tab-switching

async getUrlOfTab(tabIndex) {
return await retry.try(async () => {
log.debug(`reportingPage.getUrlOfTab(${tabIndex}`);
const handles = await browser.getAllWindowHandles();
log.debug(`Switching to window ${handles[tabIndex]}`);
await browser.switchToWindow(handles[tabIndex]);

const url = await browser.getCurrentUrl();
if (!url || url === 'about:blank') {
throw new Error('url is blank');
}

await browser.switchToWindow(handles[0]);
return url;
});
}

async closeTab(tabIndex) {
return await retry.try(async () => {
log.debug(`reportingPage.closeTab(${tabIndex}`);
const handles = await browser.getAllWindowHandles();
log.debug(`Switching to window ${handles[tabIndex]}`);
await browser.switchToWindow(handles[tabIndex]);
await browser.closeCurrentWindow();
await browser.switchToWindow(handles[0]);
});
}

async forceSharedItemsContainerSize({ width }) {
await browser.execute(`
var el = document.querySelector('[data-shared-items-container]');
Expand All @@ -73,6 +45,16 @@ export function ReportingPageProvider({ getService, getPageObjects }) {
`);
}

async getReportURL(timeout) {
log.debug('getReportURL');

const url = await testSubjects.getAttribute('downloadCompletedReportButton', 'href', timeout);

log.debug(`getReportURL got url: ${url}`);

return url;
}

async removeForceSharedItemsContainerSize() {
await browser.execute(`
var el = document.querySelector('[data-shared-items-container]');
Expand All @@ -81,9 +63,8 @@ export function ReportingPageProvider({ getService, getPageObjects }) {
`);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer use the response body, so we just return the raw response from here on out.

getRawPdfReportData(url) {
log.debug(`getRawPdfReportData for ${url}`);
let data = []; // List of Buffer objects
getResponse(url) {
log.debug(`getResponse for ${url}`);
const auth = config.get('servers.elasticsearch.auth');
const headers = {
Authorization: `Basic ${Buffer.from(auth).toString('base64')}`,
Expand All @@ -100,13 +81,7 @@ export function ReportingPageProvider({ getService, getPageObjects }) {
headers,
},
res => {
res.on('data', function(chunk) {
data.push(chunk);
});
res.on('end', function() {
data = Buffer.concat(data);
resolve(data);
});
resolve(res);
}
)
.on('error', e => {
Expand All @@ -115,6 +90,18 @@ export function ReportingPageProvider({ getService, getPageObjects }) {
});
}

async getRawPdfReportData(url) {
const data = []; // List of Buffer objects
log.debug(`getRawPdfReportData for ${url}`);

return new Promise(async (resolve, reject) => {
const response = await this.getResponse(url).catch(reject);

response.on('data', chunk => data.push(chunk));
response.on('end', () => resolve(Buffer.concat(data)));
});
}

async openCsvReportingPanel() {
log.debug('openCsvReportingPanel');
await PageObjects.share.openShareMenuItem('CSV Reports');
Expand All @@ -130,10 +117,6 @@ export function ReportingPageProvider({ getService, getPageObjects }) {
await PageObjects.share.openShareMenuItem('PNG Reports');
}

async clickDownloadReportButton(timeout) {
await testSubjects.click('downloadCompletedReportButton', timeout);
}

async clearToastNotifications() {
const toasts = await testSubjects.findAll('toastCloseButton');
await Promise.all(toasts.map(async t => await t.click()));
Expand Down Expand Up @@ -175,7 +158,9 @@ export function ReportingPageProvider({ getService, getPageObjects }) {

async setTimepickerInDataRange() {
log.debug('Reporting:setTimepickerInDataRange');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was causing Discover to return quite a few items, hitting our byte limit. I shortened the timeframe to hopefully improve test speed + get our tests running again.

await PageObjects.timePicker.setDefaultAbsoluteRange();
const fromTime = 'Sep 19, 2015 @ 06:31:44.000';
const toTime = 'Sep 19, 2015 @ 18:01:44.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
}

async setTimepickerInNoDataRange() {
Expand Down
10 changes: 3 additions & 7 deletions x-pack/test/reporting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ Reporting tests have their own top level test folder because:
### Running the tests

There is more information on running x-pack tests here: https://github.com/elastic/kibana/blob/master/x-pack/README.md#running-functional-tests. Similar to running the API tests, you need to specify a reporting configuration file. Reporting currently has two configuration files you can point to:
- test/reporting/configs/chromium_api.js
- test/reporting/configs/chromium_functional.js
- test/reporting/configs/chromium_api.js
- test/reporting/configs/chromium_functional.js

The `api` versions hit the reporting api and ensure report generation completes successfully, but does not verify the output of the reports. This is done in the `functional` test versions, which does a snapshot comparison of the generated URL against a baseline to determine success.

Expand All @@ -33,10 +33,6 @@ node scripts/functional_tests_server.js --config test/reporting/configs/[test_co
node ../scripts/functional_test_runner.js --config test/reporting/configs/[test_config_name_here].js
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed

**Prerequisites**
The reporting functional tests use [pdf-image](https://www.npmjs.com/package/pdf-image) to convert PDF's pages to png files for image comparisons between generated reports and baseline reports.
pdf-image requires the system commands `convert`, `gs`, and `pdfinfo` to function. Those can be set up by running the following.

```sh
//OSX
brew install imagemagick ghostscript poppler
Expand Down Expand Up @@ -99,7 +95,7 @@ node ../scripts/es_archiver.js --es-url http://elastic:changeme@localhost:9200 l
7. Generate some reporting URLs
- Use a mixture of Visualize, Discover (CSV), Dashboard
- Can view the current test coverage by checkout out [api/generation_urls.js](https://github.com/elastic/kibana/blob/master/x-pack/test/reporting/api/generation_urls.js). You can use different ones for better test coverage (e.g. different dashboards, different visualizations).
- Don’t generate urls from huge dashboards since this is time consuming.
- Don’t generate urls from huge dashboards since this is time consuming.
- Use dashboards that have time saved with them if you wish to have data included.
8. Save these reporting urls.
9. Navigate back to the main branch via `git checkout master`. Then create, or work off your branch as usual to add the extra test coverage.
Expand Down
2 changes: 2 additions & 0 deletions x-pack/test/reporting/configs/chromium_functional.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export default async function({ readConfigFile }) {
'["info","warning","error","fatal","optimize","reporting"]',
'--xpack.endpoint.enabled=true',
'--xpack.reporting.csv.enablePanelActionDownload=true',
'--xpack.reporting.csv.checkForFormulas=false',
'--xpack.reporting.csv.maxSizeBytes=25000000',
'--xpack.security.session.idleTimeout=3600000',
'--xpack.spaces.enabled=false',
],
Expand Down
Loading