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

Errors are not reporeted when thrown from before contexts #191

Open
jmcdo29 opened this issue Feb 16, 2022 · 3 comments · May be fixed by #196
Open

Errors are not reporeted when thrown from before contexts #191

jmcdo29 opened this issue Feb 16, 2022 · 3 comments · May be fixed by #196

Comments

@jmcdo29
Copy link

jmcdo29 commented Feb 16, 2022

Current Behavior

When using test suites, and a before throws an error, the only indication of the error being thrown is that the number of tests passed is 0 when there is 1 test in total.

Expected Behavior

The error being thrown should be bubbled up and reported as any other error would be

Minimum Reproduction

const { suite } = require("uvu");
const { ok } = require("uvu/assert");

const TestSuite = suite(
  "This suite will not report an error from the before context, just that no tests were ran"
);
TestSuite.before(() => {
  throw new Error("You will never see this");
});
TestSuite("I should be reported on!", () => {
  ok(true);
});
TestSuite.run();

You can find a reproduction of the error here

Reproduction steps

git clone
cd uvu-context-error
pnpm i / npm i / yarn
node index 

Other Information

I get the same outcome whether using uvu as a CLI (like the test script does) or calling the file directly

❯ node index.js
 This suite will not report an error from the before context, just that no tests were ran    (0 / 1)

  Total:     1
  Passed:    0
  Skipped:   0
  Duration:  1.06ms
❯ pnpm test

> uvu-before-fail@0.0.0 test ~/uvu-before-fail
> uvu . index\.js

index.js
 This suite will not report an error from the before context, just that no tests were ran    (0 / 1)

  Total:     1
  Passed:    0
  Skipped:   0
  Duration:  0.88ms
@jmcdo29
Copy link
Author

jmcdo29 commented Feb 25, 2022

I also just noticed that if there is a failure in a before then the CLI still exits with code 0 so CI doesn't register the failure.

@jmcdo29
Copy link
Author

jmcdo29 commented Feb 25, 2022

Looks like the for (hook of before) await hook(state); is not being caught properly, so the finally doesn't have anything to report on. Should this have a try/catch of its own and write to the errors like it does around line 81?

aomarks added a commit to google/wireit that referenced this issue Mar 28, 2022
aomarks added a commit to google/wireit that referenced this issue Mar 29, 2022
Adds a *watch* mode to Wireit which is used like this:

npm run <script> watch

It uses [https://github.com/paulmillr/chokidar](https://github.com/paulmillr/chokidar).

Fixes #13

Some further optimizations will come in follow ups: #49

Also
===

- Added workarounds for lukeed/uvu#191. This was masking some errors that came up during this PR.

- Refactored `ExecResult` into a class, because its API is growing.

- Added an API for killing Wireit commands spawned from the test rig. Needed because `watch` doesn't exit by itself.

- Added missing log event for a script running successfully.
@haayhappen
Copy link

This still seems to be an issue - any News?

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 a pull request may close this issue.

2 participants