Clojurians Log v2

I'll see if I can have a poke at this over the weekend. I would try doing the retries immediately inside wrap-run.

ah yeah I thought about moving everything inside wrap-run

I can try that

so you think I can just do everything with only wrap-run right?

so the thing I don't get is that I would have thought that wrap-runwould only run once, but it seems like it runs multiple times

I think so, I think that'll be the easiest path, since that way from the test type implementation's perspective it's all part of executing a single test. What you do now, calling run-testable inside post-test is going to cause some weird nesting, that might cause issues

yeah I noticed some weird interactions between my wrap-run and my post-test, even if the retries atom was modified if I only override post-test, I don't see the changes when I had both enabled

that is strange, it should run once for the test plan

(defn ^:no-gen run
  "Given a test-plan, perform the tests, returning the test results.

  Also performs validation, and lazy loading of the testable type's
  implementation."
  [testable test-plan]
  (load-type+validate testable)
  (binding [*current-testable* testable]
    (let [run (plugin/run-hook :kaocha.hooks/wrap-run -run test-plan)

uh wait no, that's probably not true

I'll check again but I was quite surprised as well, with a debugger on it was entering wrap-run 2/3 times even with a single run

ok, the docs are wrong, that run is called for each testable, so yes wrap-run is called for each testable

oh... one sec let me check something

ok, I was confusing two different things, there's the wrap-run hook, this wraps kaocha.testable/run, so in other words it wraps on the "outside" of the test type implementations

there's also :kaocha.testable/wrap, which you can set on a testable, and this will be called inisde the test type implementation

you'll have better luck with that one

(pre-test [testable test-plan]
  (update testable :kaocha.testable/wrap conj (fn [run] (fn [] ... (run) ...))))

sorry for putting you on the wrong path!

I can code up an example over the weekend if you like, this really wraps on the lowest level, i.e. executing the test var itself, so if you do your retries in there then from the point of view of the rest of kaocha it still looks like a single test.

ah nice, and sure if you could make up an example it would be great, I can also try something out now with this approach

which seems more promising

(which I think is what you would want)

and yeah it should look like a single test, however I should still track how many times it retried

but I can do that with an atom or appending something to testable I guess

I'm not sure if all of our test type implementations honor :kaocha.testable/wrap, would have to check, most probably do. I'm also not sure if it's documented 🙈

yeah just stick whatever tracking atoms you need onto the testable, or close over them in your wrapping function, you should be able to keep your state pretty localized

it's mentioned on the extending doc

note that it's :kaocha.testable/wrap, not :testable/wrap like I said before

yeah I can see it used in the capture output plugin

ok this seems better https://github.com/AndreaCrotti/kaocha-retry/blob/main/src/kaocha/plugin/retry.clj#L20

it retries as expected at least,but the reporting is still not right

do I still need to with-redefs the report function?

yeah I think the counters get confused as well

2 tests, 4 assertions, 2 failures.
#:kaocha.result{:count 2, :pass 2, :error 0, :fail 2, :pending 0}

there were only two test, and one test failed twice and then worked

but yeah probably the counter is not a big issue, reporting the test as failed is more the problem

yes, you'll still have to with-redefs the reporter, because you only want to actually report the events from the last retry. For the record counters have a look at the with-report-counters macro

not to use directly but to see what's involved, I think if you can capture t/*report-counters*, then reset them to their initial value each time you restart the test

nice amazing think it all works now

apart from the counters but maybe I'll leave them like that

to make it more explicit

ah no wait the counters seem to be fine as well now that I capture the report

ah that's great, I thought that might be the case but wasn't sure. Looking forward to your plugin, i think a lot of people will find this useful!

yeah it's published https://clojars.org/kaocha-retry-plugin and the code is: https://github.com/AndreaCrotti/kaocha-retry

however I'm not sure it's following best practices atm, but I think it's definitively usable already

one question, should you generally make a plugin do something just because it's added to the list of plugins?

or is better to still keep it disabled by default?

and also the plugins list is supposed to be just used globally in tests.edn or it makes sense to set a different plugin list for each test scenario?

Oh no you did the thing I asked you not to do... Please be mindful of naming! This is not an official Kaocha plugin so it should not be in a kaocha.* namespace. Artifact names without a prefix is also discouraged nowadays. Sorry I should've spotted this earlier but it's also explicitly mentioned in the docs for extending Kaocha.

Having a plug-in just do something when enabled isn't necessarily a problem, but it's a nice courtesy to also have a config flag for instance to change it based on the profile.

yeah sorry I'll change the name, I kind of knew it wasn't a good idea but I wasn't sure what to call it

the actual project on github and clojars is fine though right?

better like this https://github.com/AndreaCrotti/kaocha-retry/commit/df04e0d16b3e0b4208f30ff4a94e4235ed0337dd ?

actually I was trying to make the retry disabled by default, and enabling it with something like:

:tests [{:id                  :unit
          :retry.plugin/retry? true
          :test-paths          ["test"]}]}

(pre-test [testable test-plan]
    (let [max-retries (::retry-max-tries test-plan 3)
          wait-time (::retry-wait-time test-plan default-wait-time)
          retry? (::retry? testable false)
          test-id (:kaocha.testable/id testable)]

does it have to be on the test suite level? or can it be at the top level? you have access to the test-plan (i.e. the top level config), and to the current testable. If you want something in between (in this case two levels up), you would have to scan the hierarchy from the test-plan, based on the current testable.

regarding naming, both namespaces and artifacts (jars) use reverse domain name notation, to ensure that things are globally unique. That's not very strictly adhered to in the clojure world, but my philosophy is that the first one or two segments should be something you fairly uniquely "own". All our namespaces start with either lambdaisland or kaocha, we claim those names, and I don't expect anyone else to put stuff out under those names. retry is very generic, you can't really claim that, so I don't think it's a great first segment. if you have a personal domain you can use that, com.andreacrotti/kaocha-retry or something like that. Some people also use something like github-andreacrotti/kaocha-retry

ah ok yeah I thought that setting the group-id with this little tool https://github.com/applied-science/deps-library#usage was enough to set the namespace, but yeah I'll change it to andreacrotti/kaocha-retry maybe

yeah I think I'd like to make it easy to enable per test scenario

and ok I can try to scan the hierarchy then

I think the idea is to not retry all the tests, since in theory retrying could be considered a bad thing