Skip to content

refactor: improve e2e test reporting #10304

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 17, 2023
Merged

refactor: improve e2e test reporting #10304

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
54c4b2d
refactor: better e2e test reporting
aslilac Oct 16, 2023
1096c2f
🧹
aslilac Oct 16, 2023
e7d17dc
Merge branch 'main' into better-playwright-reporter
aslilac Oct 16, 2023
011dc55
report specific test failure names
aslilac Oct 16, 2023
335593a
🧹
aslilac Oct 16, 2023
91475e4
🧹
aslilac Oct 16, 2023
395a019
let's see what this looks like
aslilac Oct 17, 2023
406b6ed
Merge branch 'main' into better-playwright-reporter
aslilac Oct 17, 2023
1c6ff27
ignore some lines
aslilac Oct 17, 2023
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
Prev Previous commit
Next Next commit
let's see what this looks like
  • Loading branch information
@aslilac
aslilac committed Oct 17, 2023
commit 395a019ea13c40a1ee99555c12d0db9033763a07
3 changes: 1 addition & 2 deletions site/e2e/playwright.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export const port = process.env.CODER_E2E_PORT
? Number(process.env.CODER_E2E_PORT)
: defaultPort;

const coderMain = path.join(__dirname, "../../enterprise/cmd/coder/main.go");
const coderMain = path.join(__dirname, "../../enterprise/cmd/coder");

export const STORAGE_STATE = path.join(__dirname, ".auth.json");

Expand All @@ -15,7 +15,6 @@ const localURL = (port: number, path: string): string => {
};

export default defineConfig({
preserveOutput: "failures-only",
projects: [
{
name: "setup",
Expand Down
19 changes: 5 additions & 14 deletions site/e2e/reporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,15 @@ class CoderReporter implements Reporter {

onStdOut(chunk: string, test?: TestCase, _?: TestResult): void {
if (!test) {
const preserve = this.config?.preserveOutput === "always";
if (preserve) {
console.log(`[stdout] ${chunk.replace(/\n$/g, "")}`);
}
console.log(`[stdout] ${chunk.replace(/\n$/g, "")}`);
return;
}
this.testOutput.get(test.id)!.push([process.stdout, chunk]);
}

onStdErr(chunk: string, test?: TestCase, _?: TestResult): void {
Copy link
Member

Choose a reason for hiding this comment

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

We might be losing here output from coderd, which can be really useful during backend debugging. In the legacy version, it is logged like this:

[stderr] [unknown]: [WebServer] 2023-10-17 06:54:03.412 [info]  provisionerd.runner: apply successful  job_id=89020454-1bd8-4eed-a0c9-eb12fa532a92  template_name=68ac3052  template_version=hardcore_moser9  workspace_build_id=465c8cad-3fcb-42fb-b9bb-56007b4a1db3  workspace_id=74006ba8-82a8-4693-b971-f3da61ad02e0  workspace_name=d93260bd  workspace_owner=admin  workspace_transition=start  resource_count=1  resources="[name:\"example\" type:\"echo\"]"  state_len=0

It looks like the new reporter simply skips these log records.

Copy link
Member Author

Choose a reason for hiding this comment

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

that was sort of intentional. it adds a lot of noise when trying to debug a frontend failure, and since it's not tied to a specific test case it's hard to buffer and display later in a way that makes sense.

I also added some logic for preserveOutput, but I just read the docs and it turns out it does something entirely different from what I expected, and can't even be set from the command line like I hoped 😵‍💫

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 idea was supposed to be that backend peeps could set --preserveOutput=always when they ran the tests... I'd really like to find a compromise here to keep the output clean when possible. it's just always such a pain to find the actual error in all of the mountains of logs.

Copy link
Member Author

Choose a reason for hiding this comment

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

example run, which now has 41 occurrences of the word "error" but succeeded https://github.com/coder/coder/actions/runs/6549912328/job/17787879014?pr=10304

Copy link
Member

Choose a reason for hiding this comment

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

A good compromise would be only filtering out audit_log entries, which do not bring a lot of value. 👍

I would leave coderd output as it is helpful with debugging the request flow or provisioner behavior, especially while dealing with flaky issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

if we can also ignore lines like echo: recv done on Session session_id= error=EOF then I'll be happy. :)

Copy link
Member

Choose a reason for hiding this comment

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

Agree 👍

if (!test) {
const preserve = this.config?.preserveOutput === "always";
if (preserve) {
console.error(`[stderr] ${chunk.replace(/\n$/g, "")}`);
}
console.error(`[stderr] ${chunk.replace(/\n$/g, "")}`);
return;
}
this.testOutput.get(test.id)!.push([process.stderr, chunk]);
Expand All @@ -66,14 +60,11 @@ class CoderReporter implements Reporter {
this.timedOutTests.push(test);
}

const outputFile = `test-results/debug-pprof-goroutine-${test.title}.txt`;
const fsTestTitle = test.title.replaceAll(" ", "-");
const outputFile = `test-results/debug-pprof-goroutine-${fsTestTitle}.txt`;
await exportDebugPprof(outputFile);

const preserve = this.config?.preserveOutput;
const logOutput =
preserve === "always" ||
(result.status !== "passed" && preserve !== "never");
if (logOutput) {
if (result.status !== "passed") {
console.log(`Data from pprof has been saved to ${outputFile}`);
console.log("==> Output");
const output = this.testOutput.get(test.id)!;
Expand Down