Bool assert idea

[lpil]

What do you think?

rendered

[lpil]

@hayleigh-dot-dev and @giacomocavalieri have raised concerns with people using assert in libraries to check preconditions in functions.

I feel that introducing and documenting the feature purely in the context of testing would likely get us where we want to be, looking at Erlang and Elixir and Rust as examples of this succeeding elsewhere. I suspect most folks don't know you can even use their assertions outside of tests.

As additional encouragement we can also add another layer of nagging to gleam publish, and show when a package panics on the package index or some other documentation.

---

It was suggested we could ban panicking in published libraries, but there are lots of valid reasons to panic in libraries, even if typically it is discouraged and a bad idea. For example, OTP based code.

---

It was suggested that let assert True = could be upgraded to have all these features, but it's unwieldy to type and to read, and it fails the strangeness budget test for simple assertions, so it doesn't meet the goal of improving Gleam's test DX.

[giacomocavalieri]

It was suggested we could ban panicking in published libraries, but there are lots of valid reasons to panic in libraries, even if typically it is discouraged and a bad idea. For example, OTP based code.

Do you have any examples? Looking at gleam_otp the only place where it's used is doing some let assert Ok(thing) = ... and couldn't really be replaced with anything else.

In the tests and examples there's a couple of places doing let assert True = ...:

- let assert True = process.is_alive(p1)
+ assert process.is_alive(p1)

And there's also a couple of other assertions that could be rewritten but I'm not sure I like it:

- let assert Error(Nil) = process.receive(subject, 10)
+ assert result.is_error(process.receive(subject, 10))

- let assert Error(Timeout) = task.try_await(t3, 35)
+ assert Error(Timeout) == task.try_await(t3, 35)

What would become the preferred style in those cases? If we're not making let assert carry around the same kind of info assert does I can see people gravitating towards the latter just because it would produce more detailed error messages.
This is just my opinion but I think it's a net negative to favour bool returning functions instead of pattern matching and let assert.

[lpil]

I don't have examples to hand, but it would be any case there a precondition is checked.

The error checking example you have there should be rewritten like this:

assert process.receive(subject, 10) == Error(Timeout)

Now == is always used for equality rather than you choosing between different approaches depending on what error message you want and how much boilerplate you are willing to tolerate.

[giacomocavalieri]

But then if I want to change it in the future and assert that it's just an error do you think it would be easier to go this way:

- assert process.receive(subject, 10) == Error(Timeout) 
+ let assert Error(_) = process.receive(subject, 10)

Or this way:

- assert process.receive(subject, 10) == Error(Timeout) 
+ assert process.receive(subject, 10) |> result.is_error

To me the second one feels encouraged.

And it's a bit unexpected that writing let assert Error(Nil) = wibble will produce a different and worse error message than assert Error(Nil) == wibble.
What should happen to all let asserts that check a pattern that could be rewritten as an assert?

[lpil]

To me the second one feels encouraged.

Why do you think that? To use equality rather than a helper function is most common way to write that test both in Gleam and the other languages I'm familiar with.

And it's a bit unexpected that writing let assert Error(Nil) = wibble will produce a different and worse error message than assert Error(Nil) == wibble.

Perhaps unintuitive, but easy to learn, and it's the norm and doesn't cause problems for any of the other languages referenced. Equality is the simpler and more common feature so I don't think people would default to skipping over it in favour of the more complex partial pattern matching.

[jhillyerd]

I'm in favor of this. I think the existing gleeunit/should module is clunky to use, and find it frustrating that it does not accept custom error/context messages. This proposal allows for concise, readable, and flexible test assertions. (edit: and I like that the assertions are just gleam, rather than a DSL)

I don't think it needs to be blocked from use in non-test code, but I do expect it will lead to folks requesting a "disable assertions in production builds" feature.

[bentomas]

To me the second one feels encouraged.

I think you can discourage the second one by having whatever updated test runner that replaces the current gleeunit use the left hand side as the expected value in messaging about failures.

[sbergen]

Would it be possible to pull in the argument names, instead of 1, 2, 3 in this case?

error: Lock required for user
test/myapp/user_test.gleam:45

    assert lock_user(username, 10, role)

1. "lucystarfish"
2. 10
3. Admin

[lpil]

Argument names are not part of the public API of a function.

[sbergen]

Argument names are not part of the public API of a function.

How about labelled arguments?

[lpil]

What would you propose the exception data structure be in the presence of labels?

[sbergen]

I don't really know enough about the internals of Gleam to gauge the feasibility of this at any level, but in addition to value and kind could there be an optional label in the expression map?