Skip to content

Commit

Permalink
[Serverless][Testing] Improve serverless force logout and enable some…
Browse files Browse the repository at this point in the history
… related tests (elastic#184202)

## Summary

Looks like there are some tests being flaky because redirect after
`forceLogout` is not performed correctly on
`https://cloud.elastic.co/logout?redirectTo=%2Fprojects%2F`.
So far there has been tries to increase timeout to improve flakiness but
there are still random fails.

This PR tries to improve on this by simply adding a **retry** for the
force logout process, therefore the logout page will be reloaded, in
order to retrigger the redirect process.

There are some other cases when there was an error message in the
browser: `Failed to decode downloaded font:
https://cloud.elastic.co/449450163f45882ebfc2.woff`. This also looks
flakiness, so I'm hoping that the retry/reload mechanism helps with
those cases, too.

### For future flakiness
In case we still experience flakiness, maybe we can simply don't expect
the redirection to finish - i don't think we care about it: when we are
on this redirect page, we are already logged out.

The redirect is an external dependency from the perspective of these
tests - if [cloud.elastic.co](http://cloud.elastic.co/) cannot redirect,
it shouldn't really matter.

### Seemingly related tests

#### Skipped tests that are enabled in this PR
fixes elastic#180401
fixes elastic#184319

#### Failed but not yet skipped tests
closes elastic#174975
closes elastic#175159
closes elastic#167818
closes elastic#176413
closes elastic#175612
closes elastic#175785
closes elastic#175606
closes elastic#175599
closes elastic#175083
closes elastic#174439
closes elastic#174346
closes elastic#174566
closes elastic#173192
closes elastic#174111
closes elastic#174116
closes elastic#173950
closes elastic#173074
closes elastic#170899
closes elastic#170890
closes elastic#167597
closes elastic#169818
closes elastic#167585
closes elastic#167792
closes elastic#168751

#### Other
This test is related, and could be enabled, but since it's been skipped
for months, the business logic has changed, so the test needs to be
updated: elastic#168037

> [!Note]
> If a test fails, the issue contains its error message. If it fails
again in the future with a different error, the same issue will be
reopened, although it's not the same error. And, I searched for the
error message amongst these issues, therefore:
> - I found test fail issues containing the same error message
originally, but recently being failed on a different reason: these are
of course not included in this list as the recent error reason is
something different
> - and probably I did not find test fails that originally failed with a
different reason, but now with this, which is unfortunate

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
gergoabraham and kibanamachine authored May 31, 2024
1 parent b3a12c1 commit 22c2a67
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 57 deletions.
78 changes: 42 additions & 36 deletions x-pack/test/functional/page_objects/security_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,44 +306,50 @@ export class SecurityPageObject extends FtrService {
return;
}

this.log.debug(`Redirecting to ${this.deployment.getHostPort()}/logout to force the logout`);
const url = this.deployment.getHostPort() + '/logout';
await this.browser.get(url);

// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
if (waitForLoginPage) {
this.log.debug('Waiting on the login form to appear');
await this.waitForLoginPage();
} else {
this.log.debug('Waiting for logout to complete');
await this.retry.waitFor('logout to complete', async () => {
// There are cases when browser/Kibana would like users to confirm that they want to navigate away from the
// current page and lose the state (e.g. unsaved changes) via native alert dialog.
const alert = await this.browser.getAlert();
if (alert?.accept) {
await alert.accept();
}

// Timeout has been doubled here in attempt to quiet the flakiness
await this.retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await this.browser.getCurrentUrl();
await this.delay(1000);
const urlAfter = await this.browser.getCurrentUrl();
this.log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
const performForceLogout = async () => {
this.log.debug(`Redirecting to ${this.deployment.getHostPort()}/logout to force the logout`);
const url = this.deployment.getHostPort() + '/logout';
await this.browser.get(url);

// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
if (waitForLoginPage) {
this.log.debug('Waiting on the login form to appear');
await this.waitForLoginPage();
} else {
this.log.debug('Waiting for logout to complete');
await this.retry.waitFor('logout to complete', async () => {
// There are cases when browser/Kibana would like users to confirm that they want to navigate away from the
// current page and lose the state (e.g. unsaved changes) via native alert dialog.
const alert = await this.browser.getAlert();
if (alert?.accept) {
await alert.accept();
}

// Timeout has been doubled to 40s here in attempt to quiet the flakiness
await this.retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await this.browser.getCurrentUrl();
await this.delay(1000);
const urlAfter = await this.browser.getCurrentUrl();
this.log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
});

const currentUrl = await this.browser.getCurrentUrl();
if (this.config.get('serverless')) {
// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
} else {
return !currentUrl.includes('/logout');
}
});
}
};

const currentUrl = await this.browser.getCurrentUrl();
if (this.config.get('serverless')) {
// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
} else {
return !currentUrl.includes('/logout');
}
});
}
await this.retry.tryWithRetries('force logout with retries', performForceLogout, {
retryCount: 2,
});
}

async clickRolesSection() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import {
export default function (providerContext: FtrProviderContext) {
const { loadTestFile, getService, getPageObjects } = providerContext;

// FLAKY: https://github.com/elastic/kibana/issues/180401
describe.skip('endpoint', function () {
describe('endpoint', function () {
const ingestManager = getService('ingestManager');
const log = getService('log');
const endpointTestResources = getService('endpointTestResources');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import {
export default function (providerContext: FtrProviderContext) {
const { loadTestFile, getService, getPageObjects } = providerContext;

// FLAKY: https://github.com/elastic/kibana/issues/184319
describe.skip('endpoint', function () {
describe('endpoint', function () {
const ingestManager = getService('ingestManager');
const log = getService('log');
const endpointTestResources = getService('endpointTestResources');
Expand Down
40 changes: 23 additions & 17 deletions x-pack/test_serverless/functional/page_objects/svl_common_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,25 +147,31 @@ export function SvlCommonPageProvider({ getService, getPageObjects }: FtrProvide

await cleanBrowserState();

log.debug(`Navigating to ${deployment.getHostPort()}/logout to force the logout`);
await browser.get(deployment.getHostPort() + '/logout');

// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
// Timeout has been doubled here in attempt to quiet the flakiness
await retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await browser.getCurrentUrl();
delay(1000);
const urlAfter = await browser.getCurrentUrl();
log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
});
const performForceLogout = async () => {
log.debug(`Navigating to ${deployment.getHostPort()}/logout to force the logout`);
await browser.get(deployment.getHostPort() + '/logout');

const currentUrl = await browser.getCurrentUrl();
// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
// Timeout has been doubled to 40s here in attempt to quiet the flakiness
await retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await browser.getCurrentUrl();
delay(1000);
const urlAfter = await browser.getCurrentUrl();
log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
});

const currentUrl = await browser.getCurrentUrl();

// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
};

await retry.tryWithRetries('force logout with retries', performForceLogout, {
retryCount: 2,
});
},

async login() {
Expand Down

0 comments on commit 22c2a67

Please sign in to comment.