Fork me on GitHub
#kaocha
<
2020-12-04
>
plexus09:12:39

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

andrea.crotti09:12:17

ah yeah I thought about moving everything inside wrap-run

andrea.crotti09:12:57

I can try that

andrea.crotti09:12:11

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

andrea.crotti09:12:45

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

plexus09:12:46

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

andrea.crotti09:12:01

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

plexus09:12:15

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

plexus09:12:25

(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)

plexus09:12:44

uh wait no, that's probably not true

andrea.crotti09:12:47

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

plexus09:12:34

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

plexus09:12:02

oh... one sec let me check something

plexus09:12:17

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

plexus09:12:36

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

plexus09:12:39

you'll have better luck with that one

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

plexus09:12:54

sorry for putting you on the wrong path!

plexus09:12:24

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.

andrea.crotti09:12:18

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

andrea.crotti09:12:24

which seems more promising

plexus09:12:24

(which I think is what you would want)

andrea.crotti09:12:45

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

andrea.crotti09:12:55

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

plexus09:12:28

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 🙈

plexus09:12:00

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

andrea.crotti09:12:11

it's mentioned on the extending doc

plexus09:12:23

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

andrea.crotti09:12:50

yeah I can see it used in the capture output plugin

3
andrea.crotti10:12:15

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

andrea.crotti10:12:27

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

andrea.crotti10:12:54

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}

andrea.crotti10:12:19

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

andrea.crotti10:12:53

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

plexus10:12:06

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

plexus10:12:34

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

andrea.crotti10:12:46

nice amazing think it all works now

andrea.crotti10:12:56

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

andrea.crotti10:12:03

to make it more explicit

andrea.crotti10:12:33

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

plexus12:12:16

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!

andrea.crotti13:12:11

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

andrea.crotti13:12:40

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

andrea.crotti13:12:50

or is better to still keep it disabled by default?

andrea.crotti13:12:36

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?

plexus14:12:13

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.

plexus14:12:35

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.

andrea.crotti14:12:42

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

andrea.crotti14:12:18

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

andrea.crotti15:12:17

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"]}]}
however that config will be reachable in testable, but only in the :unit testable (which makes sense I guess). But can I access that config variable from the leaf testable as well?
(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)]

plexus15:12:12

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.

andrea.crotti15:12:48

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

andrea.crotti15:12:04

and ok I can try to scan the hierarchy then

andrea.crotti15:12:32

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

plexus15:12:44

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

andrea.crotti15:12:19

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

6