lambdaisland / kaocha

Full featured next gen Clojure test runner
https://cljdoc.org/d/lambdaisland/kaocha/
Eclipse Public License 1.0
803 stars 84 forks source link

Gracefully handle circular dependencies in watch mode #373

Closed frenchy64 closed 1 year ago

frenchy64 commented 1 year ago

Close #372

This keeps watch mode active even in the presence of circular dependencies. Basically, a tool.namespace cyclic deps error is now treated as a normal compilation error from the user's perspective.

I tested this successfully against the minimal reproduction: https://github.com/frenchy64/kaocha-circular-watch-reprod/pull/1

I tried this this on a larger project and first impressions are positive. I let the tests run through once, then intentionally added a circular dep to one of the core namespaces (depended on by dozens of files), then removed it. It behaved as expected and the process was not killed.

Full transcript ``` $ ./script/watch-checker + ./bin/kaocha :checker --watch Checking out: https://github.com/frenchy64/kaocha.git at f7d9ff428d3946eadd39f0d4dbc12cfd8a190b7f SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder". SLF4J: Defaulting to no-operation (NOP) logger implementation SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details. nREPL server started on port 59532 on host localhost - nrepl://localhost:59532 Registering annotations from typed.ann.clojure... Registering annotations from typed.lib.clojure.core.async... "Elapsed time: 6.658913 msecs" [(..........)(.)(.)(...............................)(..)(..........)(..)(....)(.......)(.......)(....)(..)(....)(........)(....)(.)(...)(.......)(.)(.)(......................................)(..................)(.....)(.................)(..)(......................)(..........................)(.......)(..........)(............................)(.......................)(.............)(..................)(.................)(..........)(.)(.........................................)(...........)(.........................................)(..............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................)(.........)(...............)(..)(..)(.)(...........................)(..)(...)(.......................)(..........................................................................)(...................)(.)(.)(.........)(..................................................................)(......................................)(.)(.....)(........)(..............................................)(..............)(.........)(.............)(................)(.)(.........)(.......)(.)(........................................................................................................................................................................)(......)(..........)(..........................................................)(.)] 536 tests, 2113 assertions, 0 failures. [E] Randomized with --seed 543413017 ERROR in checker (file:/Users/ambrose/Projects/core.typed-frenchy64/main/typed/clj.checker/src/typed/cljc/checker/type_ctors.clj:89) Failed reloading typed.cljc.checker.type-ctors: Exception: clojure.lang.ExceptionInfo: Circular dependency between typed.cljc.checker.type-ctors and typed.clj.checker.rclass-ancestor-env {:reason :lambdaisland.tools.namespace.dependency/circular-dependency, :node typed.cljc.checker.type-ctors, :dependency typed.clj.checker.rclass-ancestor-env} at lambdaisland.tools.namespace.dependency.MapDependencyGraph.depend (dependency.cljc:89) lambdaisland.tools.namespace.track$add_deps$fn__7758$fn__7762.invoke (track.cljc:26) clojure.core.protocols$iter_reduce.invokeStatic (protocols.clj:49) clojure.core.protocols$fn__8230.invokeStatic (protocols.clj:75) clojure.core.protocols/fn (protocols.clj:75) clojure.core.protocols$fn__8178$G__8173__8191.invoke (protocols.clj:13) clojure.core$reduce.invokeStatic (core.clj:6886) clojure.core$reduce.invoke (core.clj:6868) lambdaisland.tools.namespace.track$add_deps$fn__7758.invoke (track.cljc:26) clojure.core.protocols$iter_reduce.invokeStatic (protocols.clj:49) clojure.core.protocols$fn__8230.invokeStatic (protocols.clj:75) clojure.core.protocols/fn (protocols.clj:75) clojure.core.protocols$fn__8178$G__8173__8191.invoke (protocols.clj:13) clojure.core$reduce.invokeStatic (core.clj:6886) clojure.core$reduce.invoke (core.clj:6868) lambdaisland.tools.namespace.track$add_deps.invokeStatic (track.cljc:25) lambdaisland.tools.namespace.track$add_deps.invoke (track.cljc:24) lambdaisland.tools.namespace.track$update_deps.invokeStatic (track.cljc:33) lambdaisland.tools.namespace.track$update_deps.invoke (track.cljc:30) lambdaisland.tools.namespace.track$add.invokeStatic (track.cljc:73) lambdaisland.tools.namespace.track$add.invoke (track.cljc:39) lambdaisland.tools.namespace.file$add_files.invokeStatic (file.clj:86) lambdaisland.tools.namespace.file$add_files.invoke (file.clj:77) lambdaisland.tools.namespace.dir$update_files.invokeStatic (dir.clj:42) lambdaisland.tools.namespace.dir$update_files.invoke (dir.clj:36) lambdaisland.tools.namespace.dir$scan_files.invokeStatic (dir.clj:68) lambdaisland.tools.namespace.dir$scan_files.invoke (dir.clj:45) lambdaisland.tools.namespace.dir$scan_dirs.invokeStatic (dir.clj:92) lambdaisland.tools.namespace.dir$scan_dirs.invoke (dir.clj:71) lambdaisland.tools.namespace.dir$scan_dirs.invokeStatic (dir.clj:89) lambdaisland.tools.namespace.dir$scan_dirs.invoke (dir.clj:71) kaocha.watch$drain_and_rescan_BANG_.invokeStatic (watch.clj:85) kaocha.watch$drain_and_rescan_BANG_.invoke (watch.clj:83) kaocha.watch$wait_and_rescan_BANG_.invokeStatic (watch.clj:184) kaocha.watch$wait_and_rescan_BANG_.invoke (watch.clj:171) kaocha.watch$run_loop.invokeStatic (watch.clj:225) kaocha.watch$run_loop.invoke (watch.clj:198) kaocha.watch$run_STAR_.invokeStatic (watch.clj:343) kaocha.watch$run_STAR_.invoke (watch.clj:299) kaocha.watch$run$fn__8098.invoke (watch.clj:353) clojure.lang.AFn.applyToHelper (AFn.java:152) clojure.lang.AFn.applyTo (AFn.java:144) clojure.core$apply.invokeStatic (core.clj:667) clojure.core$with_bindings_STAR_.invokeStatic (core.clj:1990) clojure.core$with_bindings_STAR_.doInvoke (core.clj:1990) clojure.lang.RestFn.invoke (RestFn.java:425) clojure.lang.AFn.applyToHelper (AFn.java:156) clojure.lang.RestFn.applyTo (RestFn.java:132) clojure.core$apply.invokeStatic (core.clj:671) clojure.core$bound_fn_STAR_$fn__5818.doInvoke (core.clj:2020) clojure.lang.RestFn.invoke (RestFn.java:397) kaocha.watch$run$fn__8100.invoke (watch.clj:360) clojure.core$binding_conveyor_fn$fn__5823.invoke (core.clj:2047) clojure.lang.AFn.call (AFn.java:18) java.util.concurrent.FutureTask.run (FutureTask.java:317) java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1144) java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:642) java.lang.Thread.run (Thread.java:1589) 1 tests, 1 assertions, 1 errors, 0 failures. [watch] Error reloading, all tests skipped. [watch] Reloading #{typed.cljc.ext.clojure.core.typed__inst typed-test.clj.checker.parse-unparse typed.ext.malli.core__equalsgt typed.cljs.checker.base-env clojure.core.typed.test.overlap clojure.core.typed.load1 typed.clj.checker typed.cljc.checker.reset-env typed.cljs.checker.check-ns clojure.core.typed.test.filter-unit-tests typed.cljc.checker.check.set typed.clj.checker.reset-caches clojure.core.typed.test.kv-destructure clojure.core.typed.test.edn typed.clj.ext.clojure.core__doseq clojure.core.typed.test.javadoc typed.cljc.checker.trans clojure.core.typed.test.tap typed-test.clj.checker.check.reify typed.clj.checker.rclass-ancestor-env typed.cljc.checker.check.apply clojure.core.typed.test.ctyp-257 clojure.core.typed.test.walk typed-test.cljc.checker.update clojure.core.typed.test.namespaced-specials clojure.core.typed.test.multimethods typed.ext.clojure.spec.alpha__fdef typed.clj.generators typed.cljs.checker.check typed.cljc.checker.check.fn-method-utils typed.clj.checker.experimental.infer-vars typed.cljc.checker.check.meta-ann clojure.core.typed.test.nthnext-test clojure.core.typed.test.pprint typed.cljc.checker.check.letfn typed.clj.ext.clojure.core__defmacro clojure.core.typed.test.internal-test clojure.core.typed.test.last-test typed.cljc.checker.check.try typed-test.clj.ext.clojure.core.typed.unsafe typed.cljc.checker.cs-gen typed.cljc.checker.check.catch clojure.core.typed.all-envs typed.cljc.checker.remove typed.clj.checker.assoc-utils typed.clj.ext.clojure.core.typed.unsafe typed.cljc.checker.check.seq-ops typed.cljc.checker.check.utils typed.cljc.checker.name-utils typed.clj.ext.clojure.core__defmethod clojure.core.typed.test.reflect typed.clj.checker.check.reify typed.clj.ext.clojure.core__let typed.cljc.checker.check.special.cast typed.cljc.checker.check.recur typed-test.ext.clojure.core.typed typed.clj.ext.clojure.core__defn clojure.core.typed.test.error-msg typed.ext.clojure.core.async typed.cljc.checker.promote-demote typed.cljc.checker.check.fn typed.cljc.checker.collect-utils typed.cljc.checker.abo typed.cljc.checker.name-env clojure.core.typed.test.jvm.array clojure.core.typed.test.contract-utils-test typed.clj.ext.clojure.core__defprotocol typed.cljs.checker typed.cljc.checker.lex-env typed.cljc.checker.subst clojure.core.typed.test.ctyp-255 typed.cljc.checker.check.isa typed-test.cljc.runtime.env-utils clojure.core.typed.test.testnew clojure.core.typed.test.frozen-macros clojure.core.typed.test.tag-test typed.cljc.checker.type-ctors typed.cljc.checker.open-result typed.cljc.ext.clojure.core.typed__fn typed.clj.checker.check.host-interop typed.cljc.checker.check.local clojure.core.typed.test.set-new clojure.core.typed.test.CTYP-144 typed-test.cljc.checker.check.invoke typed-test.clj.checker.check typed.cljc.checker.check.fn-method-one clojure.core.typed.test.assoc typed.cljc.checker.init typed.cljc.checker.check.monitor typed.cljc.checker.frees typed.cljc.checker.check.special.fn typed.cljc.checker.check.if typed.clj.checker.path-type typed.cljc.checker.base-env-helper typed.clj.ext.clojure.core__ns typed.clj.ext.clojure.core__for typed.cljs.checker.test-utils clojure.core.typed.test.set-test typed-test.cljs.checker.check typed.cljc.checker.check.cli typed-test.clj.generators clojure.core.typed.test.cast typed.cljc.checker.check.fn-methods typed.cljc.checker.check.print-env typed.cljc.checker.check.ignore clojure.core.typed.test.csgen-intersection typed.clj.checker.statistics typed.clj.checker.check.deftype typed.cljc.checker.check.set-bang typed.clj.checker.check-ns typed-test.clj.checker.check.deftype clojure.core.typed.test.nth-path-elem-test typed-test.cljc.checker.name-utils clojure.core.typed.test.template typed-test.cljc.checker.check.fn-methods clojure.core.typed.test.filter-ops clojure.core.typed.test.inspector typed.clj.checker.check-form typed-test.clojure typed.cljc.checker.check.throw typed.clj.ext.clojure.core.typed__tc-ignore typed.cljc.checker.check.quote typed.cljc.ext.clojure.core.typed__pred clojure.core.typed.test.data typed.cljc.checker.local-result clojure.core.typed.base-env-helper-cljs typed.cljc.checker.check.invoke typed.cljc.checker.datatype-ancestor-env typed.cljc.checker.check-ns-common typed.clj.checker.base-env typed.cljc.checker.var-env clojure.core.typed.test.ctyp-258 clojure.core.typed.test.zip clojure.core.typed.test.browse typed.cljc.checker.check.app-error clojure.core.typed.test.rclass-supers typed.cljc.checker.check.invoke-kw typed.cljc.checker.check.nth typed.cljc.checker.check.loop typed-test.cljc.checker.check.funapp clojure.core.typed.test.field-override typed.cljs.ext.cljs.core__implements_huh typed.clj.checker.check.type-hints typed-test.cljs.checker.parse-unparse clojure.core.typed.test.string typed.clj.ext.clojure.core__fn clojure.core.typed.test.symbolic-closures typed.cljs.checker.jsnominal-env clojure.core.typed.test.core typed.clj.checker.check.method typed.cljc.checker.check.do typed.clj.checker.constant-type typed.clj.checker.array-ops typed.cljc.checker.check-form clojure.core.typed.test.mapv-test typed.cljc.checker.check.special.loop clojure.core.typed.test.subtype clojure.core.typed.test.ctyp-263 typed.cljc.doc typed.cljc.checker.filter-ops typed.clj.checker.tc-equiv typed.clj.checker.parse-unparse clojure.core.typed.test.mainnew clojure.core.typed.test.stacktrace typed.clj.checker.file-mapping typed.cljc.checker.check.map typed.cljc.checker.check-below clojure.core.typed.check.dot-cljs typed.cljc.checker.check.vector clojure.core.typed.test.ctyp-284 typed.cljc.ext.clojure.core.typed__ann-form clojure.core.typed.test.io typed.ext.clojure.spec.alpha__def clojure.core.typed.test.shell clojure.core.typed.test.cljs-core typed.cljc.checker.check.let typed.cljc.checker.check.nthnext typed.cljc.checker.runtime-check typed.cljc.checker.subst-obj typed.cljc.checker.check.const typed-test.cljs.checker.subtype typed.clj.checker.subtype clojure.core.typed.test.difference typed.cljc.checker.check.def typed.cljc.checker.inst typed.cljc.checker.check.get clojure.core.typed.test.instant typed.cljs.checker.check-form typed.cljc.checker.check.funapp-one typed-test.cljc.doc typed.cljc.checker.update typed.clj.checker.check.field typed.clj.checker.check typed.clj.checker.test-utils typed.cljc.checker.check.funapp clojure.core.typed.test.filter-expected typed.cljc.checker.check.case clojure.core.typed.test.repl-new} [(..............................................)(..)(....)(......)(.)(.)(.........................................)(....)(.............)(..................)(.)(..........)(.................)(..)(......................)(.....)(........)(...........................)(.)(.......)(..........................................................)(...........)(.......)(...............)(..........)(........)(.................)(...)(..........................................................................)(.)(............................)(.)(...................)(.)(.............)(.....)(.......................)(..............)(.........)(.........................................)(..)(..)(....)(.)(..)(......................................)(..........)(.......)(..)(.)(..................................................................)(..............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................)(......................................)(.......................)(.)(................)(..................)(..)(..........................)(...............................)(.......)(....)(........................................................................................................................................................................)(.)(.........)(.........)(.......)(..........)(.)(...)(.........)(.)(..........)] 536 tests, 2113 assertions, 0 failures. ```
codecov[bot] commented 1 year ago

Codecov Report

Base: 75.47% // Head: 75.26% // Decreases project coverage by -0.21% :warning:

Coverage data is based on head (ac7d061) compared to base (e94d84c). Patch coverage: 60.86% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #373 +/- ## ========================================== - Coverage 75.47% 75.26% -0.22% ========================================== Files 51 51 Lines 2732 2745 +13 Branches 255 257 +2 ========================================== + Hits 2062 2066 +4 - Misses 510 517 +7 - Partials 160 162 +2 ``` | Flag | Coverage Δ | | |---|---|---| | integration | `56.85% <8.69%> (-0.20%)` | :arrow_down: | | unit | `69.32% <60.86%> (-0.19%)` | :arrow_down: | 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/373?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/kaocha/watch.clj](https://codecov.io/gh/lambdaisland/kaocha/pull/373/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2thb2NoYS93YXRjaC5jbGo=) | `75.55% <60.86%> (-2.75%)` | :arrow_down: | 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.

alysbrooks commented 1 year ago

When I test against your reproduction (thanks again for that), it works when I start watch mode without a circular dependency add one, and then remove one, but if I start it with a circular dependency and then remove it, I get this:

Exception: java.io.FileNotFoundException: Could not locate circular_test__init.class, circular_test.clj or circular_test.cljc on classpath. Please check that namespaces with dashes use underscores in the Clojure file name.
 at clojure.lang.RT.load (RT.java:462)
    ...
    lambdaisland.tools.namespace.reload$track_reload_one.invokeStatic (reload.clj:35)
    lambdaisland.tools.namespace.reload$track_reload_one.invoke (reload.clj:21)
    lambdaisland.tools.namespace.reload$track_reload.invokeStatic (reload.clj:52)
    lambdaisland.tools.namespace.reload$track_reload.invoke (reload.clj:43)
    kaocha.watch$track_reload_BANG_.invokeStatic (watch.clj:61)
    kaocha.watch$track_reload_BANG_.invoke (watch.clj:60)
    kaocha.watch$plugin_pre_load_hook.invokeStatic (watch.clj:242)
    kaocha.watch$plugin_pre_load_hook.invoke (watch.clj:234)
    ...
frenchy64 commented 1 year ago

Couldn't reproduce, I tried removing the cycle in either file and both seem to have similar output:

./bin/kaocha --watch
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[E]

ERROR in unit (file:/Users/ambrose/Projects/scratch/kaocha-circular-watch/src/circular2.clj:89)
Failed reloading circular2:
Exception: clojure.lang.ExceptionInfo: Circular dependency between circular2 and circular1
{:reason :lambdaisland.tools.namespace.dependency/circular-dependency, :node circular2, :dependency circular1}
 at lambdaisland.tools.namespace.dependency.MapDependencyGraph.depend (dependency.cljc:89)
    lambdaisland.tools.namespace.track$add_deps$fn__4674$fn__4678.invoke (track.cljc:26)
    clojure.core.protocols$iter_reduce.invokeStatic (protocols.clj:49)
    clojure.core.protocols$fn__8230.invokeStatic (protocols.clj:75)
    clojure.core.protocols/fn (protocols.clj:75)
    clojure.core.protocols$fn__8178$G__8173__8191.invoke (protocols.clj:13)
    clojure.core$reduce.invokeStatic (core.clj:6886)
    clojure.core$reduce.invoke (core.clj:6868)
    lambdaisland.tools.namespace.track$add_deps$fn__4674.invoke (track.cljc:26)
    clojure.core.protocols$iter_reduce.invokeStatic (protocols.clj:49)
    clojure.core.protocols$fn__8230.invokeStatic (protocols.clj:75)
    clojure.core.protocols/fn (protocols.clj:75)
    clojure.core.protocols$fn__8178$G__8173__8191.invoke (protocols.clj:13)
    clojure.core$reduce.invokeStatic (core.clj:6886)
    clojure.core$reduce.invoke (core.clj:6868)
    lambdaisland.tools.namespace.track$add_deps.invokeStatic (track.cljc:25)
    lambdaisland.tools.namespace.track$add_deps.invoke (track.cljc:24)
    lambdaisland.tools.namespace.track$update_deps.invokeStatic (track.cljc:33)
    lambdaisland.tools.namespace.track$update_deps.invoke (track.cljc:30)
    lambdaisland.tools.namespace.track$add.invokeStatic (track.cljc:73)
    lambdaisland.tools.namespace.track$add.invoke (track.cljc:39)
    lambdaisland.tools.namespace.file$add_files.invokeStatic (file.clj:86)
    lambdaisland.tools.namespace.file$add_files.invoke (file.clj:77)
    lambdaisland.tools.namespace.dir$update_files.invokeStatic (dir.clj:42)
    lambdaisland.tools.namespace.dir$update_files.invoke (dir.clj:36)
    lambdaisland.tools.namespace.dir$scan_files.invokeStatic (dir.clj:68)
    lambdaisland.tools.namespace.dir$scan_files.invoke (dir.clj:45)
    lambdaisland.tools.namespace.dir$scan_dirs.invokeStatic (dir.clj:92)
    lambdaisland.tools.namespace.dir$scan_dirs.invoke (dir.clj:71)
    lambdaisland.tools.namespace.dir$scan_dirs.invokeStatic (dir.clj:89)
    lambdaisland.tools.namespace.dir$scan_dirs.invoke (dir.clj:71)
    kaocha.watch$run_STAR_$fn__5004.invoke (watch.clj:309)
    kaocha.watch$run_STAR_.invokeStatic (watch.clj:308)
    kaocha.watch$run_STAR_.invoke (watch.clj:294)
    kaocha.watch$run$fn__5014.invoke (watch.clj:348)
    clojure.lang.AFn.applyToHelper (AFn.java:152)
    clojure.lang.AFn.applyTo (AFn.java:144)
    clojure.core$apply.invokeStatic (core.clj:667)
    clojure.core$with_bindings_STAR_.invokeStatic (core.clj:1990)
    clojure.core$with_bindings_STAR_.doInvoke (core.clj:1990)
    clojure.lang.RestFn.invoke (RestFn.java:425)
    clojure.lang.AFn.applyToHelper (AFn.java:156)
    clojure.lang.RestFn.applyTo (RestFn.java:132)
    clojure.core$apply.invokeStatic (core.clj:671)
    clojure.core$bound_fn_STAR_$fn__5818.doInvoke (core.clj:2020)
    clojure.lang.RestFn.invoke (RestFn.java:397)
    kaocha.watch$run$fn__5016.invoke (watch.clj:355)
    clojure.core$binding_conveyor_fn$fn__5823.invoke (core.clj:2047)
    clojure.lang.AFn.call (AFn.java:18)
    java.util.concurrent.FutureTask.run (FutureTask.java:317)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1144)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:642)
    java.lang.Thread.run (Thread.java:1589)
1 tests, 1 assertions, 1 errors, 0 failures.

[watch] Error reloading, all tests skipped.
[watch] Reloading #{circular1 circular2 circular-test}
[(.)]
1 tests, 1 assertions, 0 failures.
alysbrooks commented 1 year ago

Hmm, I'm testing on Ubuntu Linux 22.04, Clojure 1.11.1, and Java 13. I can try poking around to see what might be interfering with loading the test namespace.

fdserr commented 1 year ago

Works as expected: watch keeps running when cycling on/off a circular dependency.