Skip to content

test: remove assert.doesNotThrow()

Rodrigo Muino Tomonari requested to merge github/fork/BridgeAR/remove-does-not-throw into master

There is actually no reason to use assert.doesNotThrow() in the tests. If a test threw, just let the error bubble up right away instead of first catching it and then rethrowing it. That way we get the best stack trace and normally also the best possible error message.

Someone might argue that there are rare cases (I guess about 10) where it would actually make sense to keep the current behavior. The reason would be that a loop is used to execute something and a generated error message would be thrown that is individual per entry. Otherwise it might be hard to know what entry failed.

Nevertheless, I think a better way to deal with those would be to add a console.log. And we can add those as soon as something fails.

I can not think of any other reason to keep assert.doesNotThrow(). I personally feel we might want to deprecate that function actually.

I kept the calls in the assertion test file because I work on that right now in a different PR and those mainly test assert.doesNotThrow() on its own.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Merge request reports