Skip to content

Commit

Permalink
Updates test file wrapper to deterministically detect file write comp…
Browse files Browse the repository at this point in the history
…letion (elastic#176115)

Closes elastic#119267

## Summary

Attempts to deterministically detect when a file is written in entirety
in order to resolve flaky test issues where parsed JSON is incomplete.

Flaky Test Runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5015
  • Loading branch information
jeramysoucy authored and fkanout committed Feb 7, 2024
1 parent 33ae6b7 commit 2e4c963
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Cookie', sessionCookie.cookieString())
.expect(302);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(2);
Expand Down
6 changes: 3 additions & 3 deletions x-pack/test/security_api_integration/tests/audit/audit_log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default function ({ getService }: FtrProviderContext) {

it('logs audit events when reading and writing saved objects', async () => {
await supertest.get('/audit_log?query=param').set('kbn-xsrf', 'foo').expect(204);
await retry.waitFor('logs event in the dest file', async () => await logFile.isNotEmpty());
await logFile.isWritten();
const content = await logFile.readJSON();

const httpEvent = content.find((c) => c.event.action === 'http_request');
Expand Down Expand Up @@ -68,7 +68,7 @@ export default function ({ getService }: FtrProviderContext) {
params: { username, password },
})
.expect(200);
await retry.waitFor('logs event in the dest file', async () => await logFile.isNotEmpty());
await logFile.isWritten();
const content = await logFile.readJSON();

const loginEvent = content.find((c) => c.event.action === 'user_login');
Expand All @@ -92,7 +92,7 @@ export default function ({ getService }: FtrProviderContext) {
params: { username, password: 'invalid_password' },
})
.expect(401);
await retry.waitFor('logs event in the dest file', async () => await logFile.isNotEmpty());
await logFile.isWritten();
const content = await logFile.readJSON();

const loginEvent = content.find((c) => c.event.action === 'user_login');
Expand Down
16 changes: 12 additions & 4 deletions x-pack/test/security_api_integration/tests/audit/file_wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import Fs from 'fs';
import type { RetryService } from '@kbn/ftr-common-functional-services';

export class FileWrapper {
delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

constructor(private readonly path: string, private readonly retry: RetryService) {}
async reset() {
// "touch" each file to ensure it exists and is empty before each test
Expand All @@ -32,9 +34,15 @@ export class FileWrapper {
});
}
// writing in a file is an async operation. we use this method to make sure logs have been written.
async isNotEmpty() {
const content = await this.read();
const line = content[0];
return line.length > 0;
async isWritten() {
// attempt at determinism - wait for the size of the file to stop changing.
await this.retry.waitForWithTimeout(`file '${this.path}' to be written`, 5000, async () => {
const sizeBefore = Fs.statSync(this.path).size;
await this.delay(500);
const sizeAfter = Fs.statSync(this.path).size;
return sizeAfter === sizeBefore;
});

return Fs.statSync(this.path).size > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Cookie', sessionCookie.cookieString())
.expect(302);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(2);
Expand All @@ -545,7 +545,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Authorization', `Negotiate ${Buffer.from('Hello').toString('base64')}`)
.expect(401);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Cookie', sessionCookie.cookieString())
.expect(302);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(2);
Expand All @@ -739,7 +739,7 @@ export default function ({ getService }: FtrProviderContext) {
.get(`/api/security/oidc/callback?code=thisisthecode&state=someothervalue`)
.expect(401);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(1);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/security_api_integration/tests/pki/pki_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Cookie', sessionCookie.cookieString())
.expect(302);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(2);
Expand All @@ -528,7 +528,7 @@ export default function ({ getService }: FtrProviderContext) {
it('should log authentication failure correctly', async () => {
await supertest.get('/security/account').ca(CA_CERT).pfx(UNTRUSTED_CLIENT_CERT).expect(401);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(1);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/security_api_integration/tests/saml/saml_login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Cookie', sessionCookie.cookieString())
.expect(302);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(2);
Expand Down Expand Up @@ -881,7 +881,7 @@ export default function ({ getService }: FtrProviderContext) {
})
.expect(401);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(1);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/security_api_integration/tests/token/audit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default function ({ getService }: FtrProviderContext) {
.set('Cookie', sessionCookie.cookieString())
.expect(302);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(2);
Expand Down Expand Up @@ -85,7 +85,7 @@ export default function ({ getService }: FtrProviderContext) {
})
.expect(401);

await retry.waitFor('audit events in dest file', () => logFile.isNotEmpty());
await logFile.isWritten();
const auditEvents = await logFile.readJSON();

expect(auditEvents).to.have.length(1);
Expand Down

0 comments on commit 2e4c963

Please sign in to comment.