Skip to content
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.

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

print error in tests when reading project fails #12377

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gordonwoodhull
Copy link
Contributor

@gordonwoodhull gordonwoodhull commented Mar 25, 2025

cc @cderv

When the project is invalid in a playwright test, the actual error is not printed. It just prints

Playwright tests are passing ... FAILED (1m56s)

 ERRORS 

Playwright tests are passing => ./integration/playwright-tests.test.ts:60:6
error: Error: Failed to read quarto project YAML
    throw new Error("Failed to read quarto project YAML", error);
          ^
    at findProjectOutputDir (file:///home/runner/work/quarto-cli/quarto-cli/tests/utils.ts:55:11)
    at outputForInput (file:///home/runner/work/quarto-cli/quarto-cli/tests/utils.ts:81:36)
    at cleanoutput (file:///home/runner/work/quarto-cli/quarto-cli/tests/smoke/render/render.ts:93:15)
    at fn (file:///home/runner/work/quarto-cli/quarto-cli/tests/integration/playwright-tests.test.ts:81:9)

It doesn't seem that the actual error makes it into the playwright report either.

I'm not sure if the second parameter to new Error() is valid; according to MDN it should be {cause: error}.

I tried this but still I could not find the actual error, so I suggest printing it instead.

What do you think?

(Background: I am new to Quarto projects, and quarto render will succeed if the project does not have project.type, but tests will fail. I think it would be good if the error messages indicate the actual problem.)

when the project is invalid in a playwright test, the actual error
is not printed, and it doesn't seem that this error makes it into
the playwright report either.
@gordonwoodhull gordonwoodhull requested a review from cderv March 25, 2025 20:39
@gordonwoodhull gordonwoodhull changed the title print error when reading project print error when reading project fails Mar 25, 2025
@gordonwoodhull gordonwoodhull changed the title print error when reading project fails print error in tests when reading project fails Mar 26, 2025
@cderv
Copy link
Collaborator

cderv commented Mar 26, 2025

Ok I'll have a look in that. Thanks

@@ -52,6 +52,7 @@ export function findProjectOutputDir(projectdir: string | undefined) {
// deno-lint-ignore no-explicit-any
type = ((yaml as any).project as any).type;
} catch (error) {
console.error(error);
throw new Error("Failed to read quarto project YAML", error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right this is not correct

It should be this, right ?

Suggested change
throw new Error("Failed to read quarto project YAML", error);
throw new Error("Failed to read quarto project YAML\n" + error);

This would show the error wouldn't it ? No need for a previous console.log ?
Or are you saying we need because playwright hide the error ?

Copy link
Contributor Author

@gordonwoodhull gordonwoodhull Mar 28, 2025

Choose a reason for hiding this comment

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

Looking at this more carefully, it looks like what happens is the documents still render properly and tests succeed, but then it dies in a cleanup task, only printing the stack trace but not the error.

Because we're not in Playwright at this point (I think?) the error doesn't go into the Playwright logs.

It might be better to improve the error as you say, and then catch it somewhere further up the call stack and print it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants