Skip to content

Commit

Permalink
[Reporting] Better logging for waitForSelector failure (#25762)
Browse files Browse the repository at this point in the history
* [Reporting] Better logging for waitForSelector failure

* break waitForSelector

* experimental changes

* cleanup/consistency

* fix some test report title strings

* test disable chromium

* roll back test code

* take out non-working phantom changes

* roll back disable chromium test

* allow logger to use curried tags

* temporarily re-do report failure-causing change for test

* replace newline with escaped for single log line

* undo test change

* remove obsolete test
  • Loading branch information
tsullivan authored Nov 27, 2018
1 parent 286c6a7 commit 34d7a3b
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function screenshotsObservableFactory(server) {
};

const checkForToastMessage = async (browser, layout) => {
await browser.waitForSelector(layout.selectors.toastHeader);
await browser.waitForSelector(layout.selectors.toastHeader, { silent: true });
const toastHeaderText = await browser.evaluate({
fn: function (selector) {
const nodeList = document.querySelectorAll(selector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export interface ChromiumDriverOptions {
logger: Logger;
}

interface WaitForSelectorOpts {
silent?: boolean;
}

const WAIT_FOR_DELAY_MS: number = 100;

export class HeadlessChromiumDriver {
Expand All @@ -29,7 +33,7 @@ export class HeadlessChromiumDriver {

constructor(page: Chrome.Page, { logger }: ChromiumDriverOptions) {
this.page = page;
this.logger = logger;
this.logger = logger.clone(['headless-chromium-driver']);
}

public async open(
Expand All @@ -39,7 +43,7 @@ export class HeadlessChromiumDriver {
waitForSelector,
}: { conditionalHeaders: ConditionalHeaders; waitForSelector: string }
) {
this.logger.debug(`HeadlessChromiumDriver:opening url ${url}`);
this.logger.debug(`opening url ${url}`);
await this.page.setRequestInterception(true);
this.page.on('request', (interceptedRequest: any) => {
if (this._shouldUseCustomHeaders(conditionalHeaders.conditions, interceptedRequest.url())) {
Expand All @@ -57,7 +61,7 @@ export class HeadlessChromiumDriver {
});

await this.page.goto(url, { waitUntil: 'domcontentloaded' });
await this.page.waitFor(waitForSelector);
await this.waitForSelector(waitForSelector);
}

public async screenshot(elementPosition: ElementPosition) {
Expand All @@ -84,8 +88,29 @@ export class HeadlessChromiumDriver {
return result;
}

public waitForSelector(selector: string) {
return this.page.waitFor(selector);
public async waitForSelector(selector: string, opts: WaitForSelectorOpts = {}) {
const { silent = false } = opts;
this.logger.debug(`waitForSelector ${selector}`);

let resp;
try {
resp = await this.page.waitFor(selector);
} catch (err) {
if (!silent) {
// Provide some troubleshooting info to see if we're on the login page,
// "Kibana could not load correctly", etc
this.logger.error(`waitForSelector ${selector} failed on ${this.page.url()}`);
const pageText = await this.evaluate({
fn: () => document.querySelector('body')!.innerText,
args: [],
});
this.logger.debug(`Page plain text: ${pageText.replace(/\n/g, '\\n')}`); // replace newline with escaped for single log line
}
throw err;
}

this.logger.debug(`waitForSelector ${selector} resolved`);
return resp;
}

public async waitFor<T>({ fn, args, toEqual }: { fn: EvalFn<T>; args: EvalArgs; toEqual: T }) {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/reporting/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface Logger {
debug: (message: string) => void;
error: (message: string) => void;
warning: (message: string) => void;
clone: (tags: string[]) => Logger;
}

export interface ViewZoomWidthHeight {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/test/reporting/api/chromium_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export default function ({ loadTestFile, getService }) {
await esArchiver.unload(OSS_DATA_ARCHIVE_PATH);
});

loadTestFile(require.resolve('./bwc_existing_indexes'));
loadTestFile(require.resolve('./bwc_generation_urls'));
loadTestFile(require.resolve('./usage'));
});
}
4 changes: 0 additions & 4 deletions x-pack/test/reporting/api/usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ export default function ({ getService }) {
expect(usage.reporting.enabled).to.be(true);
});

it('is using phantom browser', async () => {
expect(usage.reporting.browser_type).to.be('phantom');
});

it('all counts are 0', async () => {
reportingAPI.expectRecentPdfAppStats(usage, 'visualization', 0);
reportingAPI.expectAllTimePdfAppStats(usage, 'visualization', 0);
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/reporting/configs/chromium_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default async function ({ readConfigFile }) {
...reportingApiConfig.kbnTestServer.serverArgs,
`--xpack.reporting.capture.browser.type=chromium`,
`--xpack.spaces.enabled=false`,
`--logging.verbose=true`,
],
},
};
Expand Down

0 comments on commit 34d7a3b

Please sign in to comment.