kaocha icon indicating copy to clipboard operation
kaocha copied to clipboard

[WIP] Truncate stacktrace once the first kaocha ns is encountered

Open pesterhazy opened this issue 3 years ago • 5 comments

Fixes https://github.com/lambdaisland/kaocha/issues/58

This PR stops printing stacktraces when the first in a list of sentinels (kaocha stackframes) is encountered. Everything after that point is noise, as it doesn't pertain to the current project.

I had to scroll up on every test failure because the whole trace didn't fit in my terminal. This PR works for my small project so far, but haven't tested it extensively.

Marked as WIP because I'm not sure about a few design questions:

  • Should this be default? Optional (bring-your-own-stacktrace-printer)?
  • What if you're working on kaocha itself?
  • This is slightly "clever" logic. Should I add a unit test to the function?

pesterhazy avatar Oct 13 '22 08:10 pesterhazy

Codecov Report

Base: 75.47% // Head: 75.52% // Increases project coverage by +0.04% :tada:

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #321      +/-   ##
==========================================
+ Coverage   75.47%   75.52%   +0.04%     
==========================================
  Files          51       51              
  Lines        2732     2737       +5     
  Branches      255      255              
==========================================
+ Hits         2062     2067       +5     
  Misses        510      510              
  Partials      160      160              
Flag Coverage Δ
integration 57.10% <100.00%> (+0.07%) :arrow_up:
unit 69.52% <66.66%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/kaocha/stacktrace.clj 87.80% <100.00%> (+1.69%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

codecov[bot] avatar Oct 13 '22 09:10 codecov[bot]

On a quick skim it seems reasonable. Could you maybe paste in an example here of the output before and after this patch?

Would there be a way to opt-out of this behavior? With stuff like this (see also e.g. output-capturing, or the elision behavior we have now) I think it's important to be able to opt-out completely in case there's a concern that this cleverness is getting in people's way.

plexus avatar Oct 13 '22 13:10 plexus

before

76 lines

ERROR in unit (pico/app/control_test.clj:18)
Failed loading tests:
Exception: clojure.lang.Compiler$CompilerException: Syntax error compiling at (pico/app/control_test.clj:18:3).
#:clojure.error{:phase :compile-syntax-check, :line 18, :column 3, :source "pico/app/control_test.clj"}
 at clojure.lang.Compiler.analyze (Compiler.java:6825)
    ...
    kaocha.ns$required_ns.invokeStatic (ns.clj:13)
    kaocha.ns$required_ns.invoke (ns.clj:11)
    kaocha.type.ns$eval5680$fn__5681.invoke (ns.clj:29)
    ...
    kaocha.testable$load.invokeStatic (testable.clj:94)
    kaocha.testable$load.invoke (testable.clj:75)
    kaocha.testable$load_testables.invokeStatic (testable.clj:152)
    kaocha.testable$load_testables.invoke (testable.clj:144)
    kaocha.load$load_test_namespaces.invokeStatic (load.clj:49)
    kaocha.load$load_test_namespaces.doInvoke (load.clj:37)
    ...
    kaocha.type.clojure.test$eval7102$fn__7103.invoke (test.clj:17)
    ...
    kaocha.testable$load.invokeStatic (testable.clj:94)
    kaocha.testable$load.invoke (testable.clj:75)
    kaocha.testable$load_testables.invokeStatic (testable.clj:152)
    kaocha.testable$load_testables.invoke (testable.clj:144)
    kaocha.api$test_plan.invokeStatic (api.clj:56)
    kaocha.api$test_plan.invoke (api.clj:49)
    kaocha.api$run$fn__5358.invoke (api.clj:101)
    ...
    kaocha.api$run.invokeStatic (api.clj:99)
    kaocha.api$run.invoke (api.clj:86)
    kaocha.runner$run$fn__5421.invoke (runner.clj:132)
    ...
    kaocha.runner$run.invokeStatic (runner.clj:130)
    kaocha.runner$run.invoke (runner.clj:73)
    kaocha.runner$_main_STAR_.invokeStatic (runner.clj:176)
    kaocha.runner$_main_STAR_.doInvoke (runner.clj:144)
    ...
    kaocha.runner$_main.invokeStatic (runner.clj:187)
    kaocha.runner$_main.doInvoke (runner.clj:185)
    ...
Caused by: java.lang.RuntimeException: No such var: x/invalid
 at clojure.lang.Util.runtimeException (Util.java:221)
    ...
    kaocha.ns$required_ns.invokeStatic (ns.clj:13)
    kaocha.ns$required_ns.invoke (ns.clj:11)
    kaocha.type.ns$eval5680$fn__5681.invoke (ns.clj:29)
    ...
    kaocha.testable$load.invokeStatic (testable.clj:94)
    kaocha.testable$load.invoke (testable.clj:75)
    kaocha.testable$load_testables.invokeStatic (testable.clj:152)
    kaocha.testable$load_testables.invoke (testable.clj:144)
    kaocha.load$load_test_namespaces.invokeStatic (load.clj:49)
    kaocha.load$load_test_namespaces.doInvoke (load.clj:37)
    ...
    kaocha.type.clojure.test$eval7102$fn__7103.invoke (test.clj:17)
    ...
    kaocha.testable$load.invokeStatic (testable.clj:94)
    kaocha.testable$load.invoke (testable.clj:75)
    kaocha.testable$load_testables.invokeStatic (testable.clj:152)
    kaocha.testable$load_testables.invoke (testable.clj:144)
    kaocha.api$test_plan.invokeStatic (api.clj:56)
    kaocha.api$test_plan.invoke (api.clj:49)
    kaocha.api$run$fn__5358.invoke (api.clj:101)
    ...
    kaocha.api$run.invokeStatic (api.clj:99)
    kaocha.api$run.invoke (api.clj:86)
    kaocha.runner$run$fn__5421.invoke (runner.clj:132)
    ...
    kaocha.runner$run.invokeStatic (runner.clj:130)
    kaocha.runner$run.invoke (runner.clj:73)
    kaocha.runner$_main_STAR_.invokeStatic (runner.clj:176)
    kaocha.runner$_main_STAR_.doInvoke (runner.clj:144)
    ...
    kaocha.runner$_main.invokeStatic (runner.clj:187)
    kaocha.runner$_main.doInvoke (runner.clj:185)
    ...
1 tests, 1 assertions, 1 errors, 0 failures.

after

12 lines

ERROR in unit (pico/app/control_test.clj:18)
Failed loading tests:
Exception: clojure.lang.Compiler$CompilerException: Syntax error compiling at (pico/app/control_test.clj:18:3).
#:clojure.error{:phase :compile-syntax-check, :line 18, :column 3, :source "pico/app/control_test.clj"}
 at clojure.lang.Compiler.analyze (Compiler.java:6825)
    ...
(Rest of stacktrace elided)
Caused by: java.lang.RuntimeException: No such var: x/invalid
 at clojure.lang.Util.runtimeException (Util.java:221)
    ...
(Rest of stacktrace elided)
1 tests, 1 assertions, 1 errors, 0 failures.

pesterhazy avatar Oct 13 '22 14:10 pesterhazy

Would there be a way to opt-out of this behavior?

Totally agree, for some cases - e.g. working on kaocha itself - it might be preferable to see the full trace.

What type of option would you prefer? Plugin? Command-line switch? Any existing switch to model this on?

One possibility is to have two printers:

  • kaocha.stacktrace/print-full-cause-trace (behavior as in main)
  • kaocha.stacktrace/print-short-cause-trace (behavior as in this PR)

Then the user can select a printer using a command line switch, defaulting to kaocha.stacktrace/print-short-cause-trace

This would also allow the user to supply their own printer, without having to modify kaocha code.

pesterhazy avatar Oct 13 '22 14:10 pesterhazy

With the optional support, we could do "staged rollout": it could start out as optional and then be promoted to default when people like it

pesterhazy avatar Oct 13 '22 16:10 pesterhazy

It seems like a unit test would be awkward with the current architecture since it's printing to the terminal. Or did you just want to test sentinel-element?.

Using a printer system would be an elegant solution if users want to write their own stacktrace printer. I'm not sure how often they do. (But perhaps if we had good documentation, they would?)

alysbrooks avatar Oct 21 '22 17:10 alysbrooks

Closing in favor of https://github.com/lambdaisland/kaocha/pull/420

pesterhazy avatar Jun 15 '23 06:06 pesterhazy