feat(*) allow helpers to extend busted

[eskerda]

This change allows helpers to extend busted functionality by exposing its internals. That's the only way I found to add new blocks, happy to discuss alternatives.

See eskerda/busted-flaky for an example on how to leverage this feature.

[DorianGray]

LGTM!

[Tieske]

shouldn't this "flaky" functionality be added to the Busted core? The way it is used now is occupying the --helper option. If you need another helper for your tests, you're blocked currently.

[Tieske]

Thinking out loud here... If not in the core, can we load it from a spec file? so at the top of a spec file a call is added busted.extend("helper module name") or something similar.

[eskerda]

shouldn't this "flaky" functionality be added to the Busted core? The way it is used now is occupying the --helper option. If you need another helper for your tests, you're blocked currently.

the reason I did write it as an extension is because the implementation to block messages signaling failed is not ideal. The part that adds a third argument to blocks to define block level arguments could be interesting, though.

Shouldn't --helper flags accept arbitrary entries like busted --helper=foo --helper=bar ?

[Tieske]

How about this then:

If not in the core, can we load it from a spec file? so at the top of a spec file a call is added busted.extend("helper module name") or something similar.

[eskerda]

How about this then:

If not in the core, can we load it from a spec file? so at the top of a spec file a call is added busted.extend("helper module name") or something similar.

¯\_(ツ)_/¯ rather than fixing the actual issue ? loading the helper from the spec file is a very lousy solution.

If you want to use two helpers, it's possible to add a helper file that loads both helpers

[DorianGray]

I love the idea of adding extensions like flaky. If a spec file is using the flaky function, doesn't it makes sense to include a call to load the extension in that spec file? I feel like either making busted-flaky find busted and register itself on require or calling busted.extend at the top of the file are both good solutions.

Am I misunderstanding the requirement?

[DorianGray]

it also feels weird to modify the command line --helper to make a spec file include an additional dependency

[DorianGray]

Yet another option I just thought of is to write a helper module that loads additional defined helpers in the busted config file but :shrug:

[eskerda]

I love the idea of adding extensions like flaky. If a spec file is using the flaky function, doesn't it makes sense to include a call to load the extension in that spec file? I feel like either making busted-flaky find busted and register itself on require or calling busted.extend at the top of the file are both good solutions.

Am I misunderstanding the requirement?

One of the ideas behind busted-flaky is the possibility of using it without having to change anything on your spec files.

Tagging it and describe blocks with #flaky (or any other tag currently configured with -Xhelper --tag=foobar) wraps them on register with the flaky block. It might sound like a gimmick, but I think it really makes a point of this extension being a busted extension and not something you import into the spec files. See https://github.com/eskerda/busted-flaky#usage

Just to make sure I am not missing something, is the question if it makes sense to add a require "flaky" to all spec files on a project so the extension can be used generally?

it also feels weird to modify the command line --helper to make a spec file include an additional dependency

It's not really a dependency. It's an extension that gets "installed" before tests are run. At the time, when I went through busted --help I assumed that was already the case. I guess a valid reason against multiple --helper options would be avoiding awkward parsing -Xhelper options.

Yet another option I just thought of is to write a helper module that loads additional defined helpers in the busted config file but 🤷

That would work too. I still think the most straightforward way of dealing with @Tieske issue is recommending on my side a helper.lua file like I commented on https://github.com/Olivine-Labs/busted/pull/658#issuecomment-791787706

-- This would already load trivial helpers
require "some.helper"

local helpers = {
  -- Let's say this is a complex helper that returns a function
  require "another.helper",
  -- ...
  require "flaky",
}

return function(busted, helper, options)
  return tx_reduce(function(h, r) r and h(busted, helper, options) end, helpers, true)
end

[Tieske]

One of the ideas behind busted-flaky is the possibility of using it without having to change anything on your spec files.

Conceptually I think that if a spec file needs those helpers because it contains flaky tests, it should be explicitly in that spec file to make that clear. That's why I advocated for an extra call in the spec file, visibility, and being self-contained.

Reminds me of the _TEST global that was set in Busted 1.x, it was removed for the same reason, if your tests need it, add it explicitly. imo this is best-practice.

[eskerda]

that's a discussion on flaky-busted and not busted on itself.

An extension must apply over the whole runner and not a particular file for it to be an extension.

What if someone wanted to create an extension called busted-rollbar that sends error tracebacks to a rollbar service. Would that apply to only a set of files?

On Mon, Mar 8, 2021, 10:46 Thijs Schreijer notifications@github.com wrote:

One of the ideas behind busted-flaky is the possibility of using it without having to change anything on your spec files.

Conceptually I think that if a spec file needs those helpers because it contains flaky tests, it should be explicitly in that spec file to make that clear. That's why I advocated for an extra call in the spec file, visibility, and being self-contained.

Reminds me of the _TEST global that was set in Busted 1.x, it was removed for the same reason, if your tests need it, add it explicitly. imo this is best-practice.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Olivine-Labs/busted/pull/658#issuecomment-792628147, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABTAOFW7OSTDGG4OQUJLLDTCSMJDANCNFSM4XXB2YPA .

[Tieske]

good point @eskerda 👍

[Tieske]

@DorianGray revisiting this. I don't like the flaky implementation, since it's a hack. Yet, the changes here in Busted itself seem reasonable.

Though I'd still love to see in Busted itself:

flaky(5, function()
  -- test goes here
end)