refute_has doesn't block (for async code)

[jonleighton]

I wrote a test like this:

session
|> refute_has(css(".my-element"))

The element is becomes hidden via JS executing on the page.

My expectation was that if the element was present when refute_has first runs, it would wait for the async timeout and keep testing the DOM before raising an error. This turns out not to be true, which lead to flakey test failures.

We can do this instead:

session
|> assert_has(css(".my-element", count: 0))

But I find the behaviour of refute_has surprising. Is this deliberate? If not, perhaps it can be changed?

[mhanberg]

I believe it is deliberate, but it was written before my time. I think there has been discussion to deprecate the function entirely in favor of the method you provided, but I believe that discussion is all it's been.

[jonleighton]

Thanks for the response and moving this to a discussion. I didn't even realise GH had this feature now! 😬

One advantage for me of keeping refute_has but changing the behaviour is that it would allow us to reuse the selector for both assert_has and refute_has. On the code base I'm working on, we have "page objects" (modules), so our test code looks like this:

session
|> refute_has(MyPage.some_element())

For now we've ended up just passing the count option through:

session
|> assert_has(MyPage.some_element(count: 0))

But I don't love the fact that that exposes more of a lower level detail to the higher level code.

[mhanberg]

You can also reuse the query but modify the count like

session
|> assert_has(MyPage.some_element() |> Query.count(0))

[jonleighton]

We're introducing an assert_missing function for our tests:

  def assert_missing(parent, query) do
    assert_has(parent, query |> Query.count(0))
  end

Perhaps that's something that could be useful to add to Wallaby core?

It makes the tests read a bit nicer and means we don't need to think so much about whether or not async behaviour might trip us up.

[marcandre]

I also believe that the current refute_has should be changed:

1. It is not useful that it doesn't retry when an element is currently visible. All tests should be willing to spend max_wait_time before failing.
2. It currently systematically waits until max_wait_time before being a success. Seems clearly undesirable, and for high max_wait_time makes things unbearable.
3. I believe that it will succeed even if you pass an invalid query (e.g. refute_has(Query.css("#test", minimum: 5, maximum: 3)))

[marcandre]

Would a PR fixing this be accepted? @mhanberg

[mhanberg]

Yes, I think it would be best to approach it like this #587 (reply in thread).

Keeping the current naming, tho.

[marcandre]

Great. I hope to be able to propose something soonish (travelling right now).

[vanderhoop]

Hey @mhanberg + @marcandre Seems like this convo is a bit stale. I've got some time, and just ran into a need for this (for not the first time 😀). Will try to take it on this weekend unless I hear otherwise.

[marcandre]

Go for it @vanderhoop . I'm no longer using wallaby (we've opted to use cypress instead)