Fork me on GitHub
#polylith
<
2022-08-25
>
imre11:08:28

Just saw https://clojurians.slack.com/archives/C06MAR553/p1661423601871689 in #announcements and thought perhaps it could be of use in polylith for all the classpath munging before tests & stuff.

👍 2
ikitommi11:08:06

moving internal libraries as polylith components (e.g. library with my.common.jdbc and my.common.time ns): I guess there are two ways to do this: 1. move all utility namespaces into path which has interface and it just works as before (just different import), e.g. my.common.[jdbc|time] => my.common.interface.[util|time]. 2. create new interface ns’es that are just the public api, e.g. add my.common.interface.jdbc & my.common.interface.time. Is there an official recommendation for these or “whatever works for you”?

tengstrand13:08:46

Hi @U055NJ5CC. I would say we have three cases: 1. We have a single interface namespace where we put all out functions that we expose, and delegate to one or more implementing namespace within the component. 2. We have a single interface namespace where each function keeps its implementation in that namespace without delegating. 3. Split the interface into sub namespaces (and maybe also a "root interface") where each interface namespace is implemented like 1 or 2. Here is how I think about the different choices: 1. This is the "default" choice, because it often keeps the interface clean. 2. If the implementing functions are small, it can sometimes be cleaner to put them directly in the interface. 3. If the component contains two or more "islands of functionality" then it can be convenient to split the interface into sub interfaces, as a way to keep each part more cohesive. An example mentioned in the documentation is when we put Clojure spec code in a separate interface sub namespace. Another is the util component you mentioned.

ikitommi13:08:49

thanks!

👍 1
Eugen16:08:44

me again, still on my quest to make tests run in polylith. I found an issue that might be tested / put a warning in: Short story: we have a library and component named common (poly/common in deps.edn). This makes a confusion, even though local root is different. One masks the other. Behavior: • I started the repl via calva with: clojure -Sdeps '{:deps {nrepl/nrepl {:mvn/version,"1.0.0"},cider/cider-nrepl {:mvn/version,"0.28.5"}}}' -M:dev:test:+default -m nrepl.cmdline --middleware "[cider.nrepl/cider-middleware]" • I evaluated a file that made my tests fail • I get Execution error (FileNotFoundException) for a ns in my common library. • I ran clojure -Srepro -Stree -A:dev:test > cp.tx and saw that it was over-written by the component Should this be an issue / warning in the docs maybe?

seancorfield18:08:03

Without seeing your deps.edn files it's hard to comment for sure, but if you have some/lib in two different aliases that get combined in the Clojure CLI / t.d.a you're going to get one overwriting the other -- because deps are a hash map and that's how merge works. This is intentional so you can override library coordinates in different aliases.

Eugen18:08:29

yeah, thanks. I get that. it's easier to overwrite with poly/component-name though because usually libs have unique names. easier in my head at least. Wondering if this should be specified in the docs

seancorfield18:08:26

You can't have multiple bricks with the same name and all your bricks should be poly/<some-name> so you should not use poly/something for anything that isn't a brick.

seancorfield18:08:33

(but it's just a convention)

Eugen19:08:28

hi, so I finally made some progress and I have tests running from the bases. Feeling better and better about the tool 🙂 . Thanks. I am currently looking into enabling rcf tests in a polylith project: https://github.com/hyperfiddle/rcf/issues/62 . If you have any info on this, please let me know. I will make PR's to document it.

seancorfield20:08:33

You can specify per-project setup/teardown fixtures in workspace.edn -- which is what we do at work to run code before/after each project's test run.

Eugen20:08:31

thanks @U04V70XH6 I was/am looking for that. I need to run (hyperfiddle.rcf/enable!) before the tests start. Found docs on the Testing page 🙂

Eugen20:08:26

my setup function runs, but RCF tests are not run if the files are under src/ . I added a rcf test under test/ and it runs.

Eugen20:08:03

dre.app.sample-rcf-test(
  RCF__4
  RCF__4✅✅
)]

Eugen21:08:53

hey @U08BJGV6E any idea about the above? why namespaces under src/ are not found by hyperfidle/rcf ? I have a hunch this is related to how polylith test runner (using polylith-kaocha) handles the paths.

Eugen21:08:53

I have rcf tests in normal namespaces and I would like to run them during tests. it does work if the rcf tests are in a test file under test/

Eugen21:08:57

so I think I found out why: it only considers files that end with _test . and only under test/ . I added a test file on src/ and another file without test on test/ . Only files ending in _test under test/ works. I wonder if this is something I can change with kaocha config

imre06:08:09

Check out the ns-patterns config for kaocha

Eugen12:08:38

hi, tried this config and it finds tests and RCF tests without -test on test/ path but not RCF tests

#kaocha/v1
{:tests [{:id          :unit
          :test-paths  ["test" "src"]
          :ns-patterns [".*"]}]
 :plugins [:kaocha.plugin/junit-xml]
 :reporter [kaocha.report/documentation
            kaocha.report/dots]
 :kaocha.plugin.junit-xml/target-file "target/junit-app.xml"}

imre12:08:22

I think you’ll need a kaocha plugin for that

imre12:08:34

I don’t know whether one exists

Eugen12:08:35

it finds RCF tests as well, updated the above

Eugen12:08:16

I know RCF tests work on src as well, I tried it without polylith . will try again

Eugen12:08:35

> I think you’ll need a kaocha plugin for that what do you mean?

imre12:08:38

Let’s take a step back as I’m not sure I understand you now. What RCF tests does it find? What RCF tests does it not find?

Eugen12:08:58

Running tests for the app project using test runner: polylith-kaocha...
--- unit (clojure.test) ---------------------------[
dre.app.sample-rcf-test(
  RCF__4
  RCF__4✅✅
)
dre.app.sample-test(
  test-sample
    Test example.
)
dre.app.sample(
  RCF__4
  RCF__4✅✅
)]

Eugen12:08:13

all of them are on test/ classpath

imre12:08:02

so these are the ones it finds, right?

Eugen12:08:08

I have another RCF test on src/ that is not found

imre12:08:36

how are these tests defined in code?

Eugen12:08:49

(ns dre.analytics.sample
  (:require [hyperfiddle.rcf :refer [tests]]))

(tests
 
 "another one bites the dust"
 (= 1 1) := true
 
 (+ 1 1) := 2
 )

imre12:08:24

is this under src or test?

imre12:08:07

and how do the ones found under test look?

Eugen12:08:21

same file on test

imre12:08:32

same namespace name too?

Eugen12:08:50

see names above in test output

imre12:08:45

are the ones not found in a component/base/project?

Eugen12:08:57

in the same base

Eugen12:08:05

all files

imre12:08:33

try removing :test-paths ["test" "src"] from the kaocha config

Eugen12:08:17

#kaocha/v1
{:tests [{:id          :unit 
          :ns-patterns [".*"]}]
 :plugins [:kaocha.plugin/junit-xml]
 :reporter [kaocha.report/documentation
            kaocha.report/dots]
 :kaocha.plugin.junit-xml/target-file "target/junit-app.xml"}
no change

imre12:08:26

run the tests with :verbose and look for a log line that starts with [polylith-kaocha] Evaluating in project:

imre12:08:54

you should find :src-paths and :test-paths in that line

imre12:08:32

My assumption is that the path to your tests under src would only appear under src-paths, is that correct?

Eugen12:08:17

yeah, you are right

Eugen12:08:32

src path contains all paths

imre12:08:50

is there anything under test-paths?

imre12:08:04

I’m assuming the paths to the tests the runner does find

Eugen12:08:04

test path contains only the test paths

imre12:08:54

Okay, that’s the problem you got there. From your snippet it appears the tests are being discovered by Kaocha’s (clojure.test) test type discoverer.

imre12:08:06

That discoverer only looks at test-paths

imre12:08:50

Fortunately there’s a way to fix that. Outside of polylith, you’d set :test-paths ["test" "src"] in the config

imre12:08:06

But inside it’s more complicated as poly forwards paths to kaocha

imre12:08:30

There’s a hook in polylith-kaocha to modify the kaocha config before it’s passed to kaocha: https://github.com/imrekoszo/polylith-kaocha/blob/182e88dfb3aebcc4be321130fd7d0e6e77beafc6/workspace.edn#L20

imre12:08:57

for projects where this matters, you can add a custom fn that modifies the config so src-paths get added to test-paths or something like that

Eugen12:08:33

thanks, I think I can probably manage. let me see. one I figure this out, would it make sense to have a switch / special feature to suport this OOTB?

imre12:08:27

I could see p-k having some ready-made hook implementations like this that one would just then have to reference from workspace.edn yes

👍 1
imre12:08:53

But I’m going to be cautious and say that first there needs to be a proper problem statement

imre12:08:35

Also, there’s nothing preventing anyone from creating a new github project that provides some of these

Eugen12:08:52

your project, your rules. I like RCF tests for unit tests because: • tests are local to code they test • they can be used as docs.

Eugen12:08:36

having some support for them OOTB with little effort would be nice

imre12:08:38

But since the problem you’re facing is not entirely poly-specific, I think the best solution would be a plugin to kaocha itself, which could discover RCF tests properly

imre12:08:58

for example, there’s the kaocha OOTB plugin that https://cljdoc.org/d/lambdaisland/kaocha/1.68.1059/doc/automatic-spec-test-check-generation - that is written in a way that it looks at the src-paths for fdefs

imre12:08:16

Having that as a kaocha plugin would benefit users of RCF tests regardless of whether they use poly or not

Eugen12:08:49

thanks for the insight, will look into it

imre12:08:02

So I’d probably start a discussion in #kaocha about it. Perhaps it’s easier than you think

imre12:08:48

You’re welcome. Let me know if this was indeed the issue

👍 1
Eugen14:08:32

so I think this works, but I am seeing some issues - probably because are using the same namespace in 2 bases. Because all bases are added to the classpath, I think this is an issue. Will try to solve:

(defn post-enhance-config [config opts]
  (println "custom post-enhance-config")
  (let [res (config/default-post-enhance-config config opts)
        {:keys [kaocha/tests]} res
        tests-config (first tests)
        {:keys [kaocha/test-paths kaocha/source-paths]} tests-config
        merged-paths (into [] (concat test-paths source-paths))
        new-tests (assoc tests-config :kaocha/test-paths merged-paths)]
    (assoc res :kaocha/tests [new-tests])))

Eugen14:08:07

I get namespace xxxx not found, though the ns should be on the classpath . and that namespace is singular in all files across all bases/ components

imre14:08:31

I'd probably reverse the logic a bit and do path magic before calling default-post-enhance-config

Eugen14:08:20

thanks, this makes the logic smaller, but I still get an error in processing:

(defn post-enhance-config [config opts]
  (println "custom post-enhance-config")
  (let [{:keys [src-paths test-paths]} opts
        opts (assoc opts
                    :test-paths
                    (into [] (concat src-paths test-paths)))]
    (config/default-post-enhance-config config opts)))

Eugen10:08:00

so I got RCF tests to work by passing the JVM options via :poly alias. I pushed the code to the PR and will clean the project a bit before requesting a merge. Thanks @U08BJGV6E and everybody for lending a hand. @U08BJGV6E: Would you consider merging the PR? I would like to add some documentation specific to RCF and remove some files before the PR can be squash-merged .

imre11:08:35

What is the minimal solution needed in the end? JVM options only? Or JVM options plus src+test-paths?

Eugen11:08:41

will discover that and document. I need to finish something for work. Right now I am happy it works 🙂

2
imre11:08:21

if it’s the former then I don’t think that has anything to do with p-k - it should also work with poly oob

👍 2
imre11:08:05

if it’s the latter, then p-k only matters as much as it’s a test runner that is capable of discovering tests from src-paths

imre11:08:42

(as the rest is done by rcf + jvm opts)

imre11:08:58

In any case I don’t think this strongly relates to p-k so I think a separate example repo would demonstrate it better (without all the p-k cruft): just the minimal solution to get rcf+poly work. Then we could link to that repo from p-k and potentially elsewhere (rcf, poly etc)

Eugen13:08:25

so I managed to create a repro for src/ as well. https://github.com/ieugen/poly-rcf . I needed to add "src" to :test extra-paths for it to work. So these 2 things are needed: jvm option, src paths need to be added to test classpath so converting rfc tests to clojure.test works

imre13:08:24

Nice, no need for p-k at all. Thank you

Eugen13:08:13

I would still ike to use Kaocha - because IMO it has better reporting.

2
imre13:08:44

Sure. I meant not needed to get rcf working with poly