Fork me on GitHub
#polylith
<
2024-01-29
>
Stefan07:01:22

Hi, we're using the latest from git for the poly tool. Tomorrow I did an update (pulling in these changes: https://github.com/polyfy/polylith/compare/22098d6bfd2ea4bd5f52df03b5260c8c9e079f9d...aece34ab40255fd40038abbff79433fdf7cd5759) and now I get this when I run my tests:

Cannot invoke "clojure.lang.IFn.invoke(Object)" because "project_to_projects_to_test" is null
We're using the kaocha test runner. It seems that removing that config from workspace.edn makes it run again. Maybe related to the posts above by @seancorfield?

tengstrand08:01:33

Yes, the 0.2.19-SNAPSHOT introduced a breaking change, so the Kaocha doesn't work right now. @U08BJGV6E is working on a fix, so that it will support both the old format and 0.2.19. This will probably be fixed this week I think. In the meantime (if you have converted your workspace) you should be able to have both the old :projects key in workspace.edn with your test configs and the project config.edn files (the tool can read both the old and the new format), as a work-around.

👍 1
Stefan09:01:19

It seems we missed something about converting workspaces, I'll look into that as well 🙂

Stefan09:01:26

Thanks for the heads-up!

imre09:01:47

@U1G0HH87L have you documentation about that workspace vs config.edn change?

tengstrand09:01:24

Yes, the new snapshot documentation is updated, e.g. the https://cljdoc.org/d/polylith/clj-poly/0.2.19-SNAPSHOT/doc/project doc.

imre09:01:32

I meant a doc about converting your workspace to the new format

tengstrand09:01:46

You can see the changes https://cljdoc.org/d/polylith/clj-poly/0.2.19-SNAPSHOT/doc/versions. Keys have basically moved from project-to-... keys to each project.

imre09:01:05

perfect, thank you!

👍 1
Stefan11:01:57

Can poly use a different file name than config.edn? We already have config.edn files in some of our project dirs...

tengstrand11:01:45

I can make that configurable.

Stefan12:01:44

I think that might be a good idea. config.edn is a quite generic name, high risk of conflict. Normally you see those in subfolders (`.lsp/config.edn`, .clj-kondo/config.edn), then the risk of conflict is not there.

Stefan12:01:27

I'm not saying that they should be in subfolders for poly by the way, but having a way to change that (file) name seems like a good idea.

tengstrand12:01:20

My original idea was to use project.edn , base.edn , and component.edn. Maybe we should use that naming pattern instead.

tengstrand12:01:57

Then we probably don't have to support configuring the name of the config files.

imre12:01:17

Suggestion: use the same filename everywhere but make it poly specific. Config, project, component are too generic

imre12:01:34

Same file name: they wont appear in the same folder anyway

Stefan12:01:01

There are pros and cons for either. One difference (positive or negative depending on your taste) is that having the same file name causes there to be multiple files with the same name in your editor. Also with deps.edn, I have so many in my project. In practice due to good completion behavior of vscode this is not a problem for me.

tengstrand12:01:58

If we make it configurable, then we can maybe leave it as it is.

1
👍 1
imre23:01:56

@UGNFXV1FA can you try aece34ab40255fd40038abbff79433fdf7cd5759 of polylith-kaocha and let me know if it works?

imre07:01:20

Sorry, wrong commit sha

Stefan07:01:07

That seems to work. I do get tons of debug level logging, which I believe (but am not sure!) was disabled before using logback config. But that can very well be a problem on my end. Is that something that might be explained by your changes you think?

imre07:01:49

I haven't made changes to logging

Stefan07:01:15

No I was more thinking in terms of search paths / class paths or something.

Stefan07:01:09

It's probably us who's causing this, not you.

imre07:01:01

I did bump some deps but they are mostly just poly and kaocha so it could be on your side yes

imre07:01:09

Thanks for confirming it look okay

imre07:01:28

I'll do an official release later today hopefully

Stefan07:01:35

Putting another copy of logback.xml in my project's resources folder fixes the logging. This is a downside of poly really, that you get lots of duplication in deps.edn files and resources like that logback.xml and build.clj if you're not careful and clever about it.

imre07:01:19

Multiple build.clj files are somewhat surprising to me

Stefan07:01:44

That's probably us being not clever enough then 😅. It was a LONG time ago since we set this up, maybe things have changed since then. We have a build.clj symlink in each project folder. They all link to the same file, but that way we can do clojure -T:build uberjar ... in each project folder.

imre07:01:29

Sounds legit tho

👍 1
Stefan07:01:51

So I assume you're going to make a new release of the kaocha wrapper/runner one of these days right? Then I'll remove the explicit sha again and wait for your release, we're not in a hurry.

imre07:01:52

I'll either tag this commit or add one on top really

👍 1
imre07:01:24

Unless issues surface till then

tengstrand07:01:18

I can confirm that a38bd8b works with 0.2.19-SNAPSHOT. I will have another look again tonight.

👍 1
tengstrand08:01:27

@UGNFXV1FA An alternative is to have a single build.clj file at the workspace root. This is how the polylith repo handles it.

Stefan08:01:52

@U1G0HH87L That's good to know, I'll try that, thanks!

👍 1
seancorfield11:01:49

We have a single build.clj file at work and build two dozen projects with it in our Polylith repo. Every project does have its own logging config file tho'...

seancorfield11:01:34

And we have about 200 deps.edn files:grin:

Stefan11:01:26

🙂 I guess deps.edn is the new package.json 🙂

tengstrand18:01:47

I guess I'm doing something wrong @U08BJGV6E. I have create the https://github.com/polyfy/polylith/tree/prepare-0219/examples/test-runners example project. When I execute clojure -M:poly test :all project:kaocha-override-global from the workspace root, it can't find the kaocha-wrapper. Maybe someone can have a look.

imre18:01:53

Projects tested with kaocha need a test dep on kaocha wrapper

tengstrand18:01:21

Okay, will have a try.

seancorfield18:01:30

Interesting. My external test runner can be added as part of the :poly dependency in the workspace deps.edn file...

imre18:01:23

Kaocha needs to be in-classloader

tengstrand18:01:35

Now it works, thanks!

👍 1
imre18:01:26

Because it replaces the clojure.test/run-tests part

seancorfield18:01:35

Ah, right... whereas mine still leverages all the standard clojure.test stuff and only needs to add just its "main" ns source to the classpath in order to run the tests...

imre18:01:54

And the -wrapper around it understands how to initialize kaocha based on the instructions given by the poly-tool side plugin

Stefan14:02:37

Thanks for the heads-up @U1G0HH87L. I noticed that this was now also reverted and removed from the release right?

tengstrand15:02:34

Yes, the configuration of projects and base is moved back to workspace.edn but the internal move of (mostly) project settings in the workspace structure is kept as it is.

👍 1
tengstrand15:02:23

So it should be safe to use "as is" except if you use another test runner than the default one!

imre15:02:33

is config in config.edn files still supported or did that get removed entirely?

imre15:02:00

(I updated p/k to config.edns when I caught up with the previous breaking change)

Stefan15:02:14

So it should be safe to use "as is" except if you use another test runner than the default one!So with kaocha we do need to change something?

tengstrand15:02:07

config.edn files have been rolled back entirely, so for those who has migrated already, they (unfortunately) have to rollback those changes (delete all config.edn files and move the config back to workspace.edn.

imre15:02:56

I'll try to remember to do that when I have the time

imre15:02:04

@UGNFXV1FA it shouldn't affect consumers

👍 1
seancorfield17:02:20

> they (unfortunately) have to rollback those changes This is when small commits in git are so useful 😉

Stefan18:02:48

Totally agree! Stop squashing those branches peops! :)