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

Add capturing bypass #366

Merged
merged 6 commits into from Jan 20, 2023
Merged

Add capturing bypass #366

merged 6 commits into from Jan 20, 2023

Conversation

plexus
Copy link
Member

@plexus plexus commented Dec 4, 2022

When using the output-capturing plugin, it is sometimes desirable to bypass it and still print things directly.

See https://clojurians.slack.com/archives/CCY2V0U6A/p1670084873985659 for a workaround.

This applies the same logic, but provides a more generic bypass macro.

It seems FileDescriptor is not available in babashka, so until that changes this will be Clojure only.

@plexus plexus added the improvement Incremental improvement of an existing feature label Dec 4, 2022
@codecov
Copy link

codecov bot commented Dec 4, 2022

Codecov Report

Base: 75.21% // Head: 75.42% // Increases project coverage by +0.20% 🎉

Coverage data is based on head (1c9ada5) compared to base (295791e).
Patch coverage: 33.33% of modified lines in pull request are covered.

❗ Current head 1c9ada5 differs from pull request most recent head 1266ab0. Consider uploading reports for the commit 1266ab0 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
+ Coverage   75.21%   75.42%   +0.20%     
==========================================
  Files          51       51              
  Lines        2740     2734       -6     
  Branches      256      258       +2     
==========================================
+ Hits         2061     2062       +1     
+ Misses        518      510       -8     
- Partials      161      162       +1     
Flag Coverage Δ
integration 57.00% <0.00%> (+0.03%) ⬆️
unit 69.45% <33.33%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
src/kaocha/core_ext.clj 64.28% <ø> (ø)
src/kaocha/plugin/capture_output.cljc 96.87% <0.00%> (ø)
src/kaocha/plugin/notifier.clj 82.82% <ø> (ø)
src/kaocha/plugin/randomize.clj 95.23% <ø> (ø)
src/kaocha/runner.clj 46.80% <ø> (ø)
src/kaocha/type/var.clj 83.33% <ø> (-1.29%) ⬇️
src/kaocha/watch.clj 78.30% <ø> (+4.44%) ⬆️
src/kaocha/testable.clj 82.23% <100.00%> (ø)
src/kaocha/type/ns.clj 95.74% <0.00%> (-2.13%) ⬇️
... and 3 more

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

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

@alysbrooks
Copy link
Member

Yeah, I had a conversation with Michiel Borkent when there was an earlier issue with FileDescriptor, and he said FileDescriptor probably wasn't a good candidate for addition to Babashka.

Copy link
Member

@alysbrooks alysbrooks left a comment

Choose a reason for hiding this comment

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

Based on the Slack conversation, it sounds like this might be intended for using in test code by users of Kaocha? In that case, I think we'd ideally have documentation for it.

@@ -12,7 +12,7 @@
[kaocha.result :as result]
[kaocha.specs :refer [assert-spec]]
[kaocha.util :as util])
(:import [clojure.lang Compiler$CompilerException]))
(:import (clojure.lang Compiler$CompilerException)))
Copy link
Member

Choose a reason for hiding this comment

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

What's the rationale for using parentheses over brackets? Is it just to indicate the different semantics between requires and imports?

Copy link
Member

Choose a reason for hiding this comment

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

To be clear, not arguing against this change, just wondering.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason is that the first item in the list is special. In this case I agree with Alessandra Sierra https://stuartsierra.com/2016/clojure-how-to-ns.html

The practical result is that you get more sensible indentation

(:import (foo.bar Klass
                  OtherKlass))

(:import [foo.bar Klass
          OtherKlass])

@plexus plexus merged commit 29f98cc into main Jan 20, 2023
@plexus plexus deleted the add-capturing-bypass branch January 20, 2023 12:46
@plexus
Copy link
Member Author

plexus commented Jan 20, 2023

Released in v1.75.1190

[lambdaisland/kaocha "1.75.1190"]                 ;; deps.edn
{lambdaisland/kaocha {:mvn/version "1.75.1190"}}  ;; project.clj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Incremental improvement of an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants