Wrap executing test to match clojure.test for fixtures

[NoahTheDuke]

Fixes #367

The fix is to wrap the existing test execution logic in an anonymous function, and use that instead of the given test function itself so that any thrown exceptions are caught by the test wrapping logic and not by an :each fixture.

[codecov[bot]]

Codecov Report

Base: 75.26% // Head: 75.32% // Increases project coverage by +0.06% :tada:

Coverage data is based on head (0f74774) compared to base (7d6c897). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #368 +/- ## ========================================== + Coverage 75.26% 75.32% +0.06% ========================================== Files 51 51 Lines 2733 2736 +3 Branches 256 256 ========================================== + Hits 2057 2061 +4 + Misses 515 514 -1 Partials 161 161 ``` | Flag | Coverage Δ | | |---|---|---| | integration | `57.07% <84.21%> (+0.04%)` | :arrow_up: | | unit | `69.50% <78.94%> (+0.17%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/lambdaisland/kaocha/pull/368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/kaocha/type/var.clj](https://codecov.io/gh/lambdaisland/kaocha/pull/368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2thb2NoYS90eXBlL3Zhci5jbGo=) | `84.61% <100.00%> (+1.28%)` | :arrow_up: | | [src/kaocha/type/ns.clj](https://codecov.io/gh/lambdaisland/kaocha/pull/368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2thb2NoYS90eXBlL25zLmNsag==) | `97.87% <0.00%> (+2.12%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[NoahTheDuke]

Just in case it's not clear, the change is relatively small, it just looks big because of indentation/whitespace changes. If you toggle showing whitespace, you'll see it's "merely" moving the bulk of testable/-run :kaocha.type/var into a separate function and then wrapping that in the original. Not to say you shouldn't examine this thoroughly, just to make my intent clear.

[alysbrooks]

Yeah, I tried using difftastic and it reduces the diff considerably. (Although even that tool doesn't point out that most of the remaining changes are really "moves"). My comment about wanting another set of eyes was more me wondering about consequences to this change I didn't think of, as this code path is hit for a large number of tests.

[NoahTheDuke]

Hope y'all had a nice holiday break. Is there anything I need to do to get this doubly reviewed?

[alysbrooks]

@NoahTheDuke Sorry for the delay. We've been on break this week as well.

There's not anything you need to do to get that review. It might be good to have a more spelled-out process for getting review, but right now we the maintainers contact reviewers internally. Normally, a single review pass is enough; this PR is just a small change I suspect could have subtle implications.

[NoahTheDuke]

Cool, thanks @alysbrooks. I don't mind being patient, just want to make sure you're not actually waiting on me for something I missed.

[plexus]

I'm having another look at the code, and I think this might interfere with --fail-fast. Not 100% sure. @NoahTheDuke or @alysbrooks would either of you have a moment to verify that fail-fast still works as expected? Thanks!

[NoahTheDuke]

By "interfere", do you mean that it would exit kaocha with an exception and stack trace instead of reporting a fail? In both cases, the test run is stopped immediately.

For example, I copied the contents of my fixtures/a-tests/baz/qux_test.clj file into test/unit/koacha/report_test.clj file and ran it with --fail-fast:

$ bin/kaocha --focus kaocha.report-test --fail-fast
--- unit (clojure.test) ---------------------------
kaocha.report-test
  result-failures-test
  dots*-test
  doc-test
  assertion-type-test
  result-test
  fail-fast-test
  dispatch-extra-keys-test
    it dispatches to custom clojure.test/report extensions
    it does nothing if there is no matching multimethod implementation
    it does nothing if it's a key known to Kaocha
    it does nothing if the key is globally marked as "known"
  testing-vars-str-test
    getting info from testable
    explicit file/line override
    clojure.test legacy compatiblity
  print-output-test
  tap-test
  fail-summary-test
  print-expr-test
  doc-print-contexts-test
  nested-test ERROR

Randomized with --seed 1538461704

ERROR in kaocha.report-test/nested-test (report_test.clj:107)
Uncaught exception, not in assertion.
Exception: java.lang.Exception: fake exception
 at kaocha.report_test$fn__10304.invokeStatic (report_test.clj:107)
    kaocha.report_test/fn (report_test.clj:106)
    ...
    clojure.main.main (main.java:40)
14 tests, 43 assertions, 1 errors, 0 failures.

Top 1 slowest kaocha.type/clojure.test (0.12594 seconds, 96.9% of total time)
  unit
    0.12594 seconds average (0.12594 seconds / 1 tests)

Top 1 slowest kaocha.type/ns (0.11073 seconds, 85.2% of total time)
  kaocha.report-test
    0.00738 seconds average (0.11073 seconds / 15 tests)

Top 3 slowest kaocha.type/var (0.05006 seconds, 38.5% of total time)
  kaocha.report-test/result-failures-test
    0.02049 seconds kaocha/report_test.clj:247
  kaocha.report-test/fail-summary-test
    0.01609 seconds kaocha/report_test.clj:180
  kaocha.report-test/tap-test
    0.01348 seconds kaocha/report_test.clj:324

bin/kaocha --fail-fast --focus 'kaocha.report-test/nested-test'

The purpose of my change is to expose these, to not swallow exceptions in test functions. If the "swallowing exceptions" behavior something you want from Kaocha then this change isn't appropriate and I'll just have to maintain my own fork.

Thanks so much for the review.

[plexus]

All good then, I just wanted to make sure since fail fast internally uses exceptions.

[plexus]

[NoahTheDuke]

Ayyyyy thank you!