Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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,
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably don't want the bang here:

fn: () => document.querySelector('body').innerText,

Copy link
Member Author

Choose a reason for hiding this comment

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

The ! is the "Non-null assertion operator" for Typescript to not fail compilation for the reason that the return from document.querySelector('body') is possibly null. It's not as safe as a guard, though. It's possible for querySelector to return null if an element isn't found, but for body I don't see it ever happening.

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL! Does the TS compiler remove this when built? Reason I ask is because this function gets stringified and sent over WebSockets over to the browser... so if it doesn't it'll have issues running

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 @@ -26,6 +26,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);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

:)

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
2 changes: 1 addition & 1 deletion x-pack/test/reporting/configs/chromium_functional.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default async function ({ readConfigFile }) {
return {
...functionalConfig,
junit: {
reportName: 'X-Pack Chromium API Reporting Tests',
reportName: 'X-Pack Chromium Functional Reporting Tests',
},
testFiles: [require.resolve('../functional')],
kbnTestServer: {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/reporting/configs/phantom_functional.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default async function ({ readConfigFile }) {
return {
...functionalConfig,
junit: {
reportName: 'X-Pack Phantom API Reporting Tests',
reportName: 'X-Pack Phantom Functional Reporting Tests',
},
testFiles: [require.resolve('../functional')],
kbnTestServer: {
Expand Down