Assert on redirect?

[rgraff]

Is it possible to assert after a redirect what the current uri is?

[sodapopcan]

I also use this all the time in LiveViewTest. The API might be a little tricky in PhoenixTest since function like click_link can take 1 or 2 string params, click_link(session, "click me") and click_link(session, "a", "click me"). I think an options list would have to be added. Maybe something like this:

click_link(session, "click me", to: ~p"/resulting/path")

It could also be assert_path: ~p"/resulting/path" but I kind of like :to since it's more concise and, in context, it reads as an expectation.

I'm sure there are other ways, though.

PS, apologies for all the noise I've made today, @germsvel

EDIT: Probably should have used submit_form as the example as that is where follow_redirect is most used in which case it could maybe just be an extra parameter. Apologies, I should have thought on this a bit more before responding.

[rgraff]

I think click_link(session, "click me", to: ~p"/path") would be valuable as a matcher (e.g. click the "click me" link that goes to "/path") but it's limited and not the use case I was thinking of.

Here's an example where I want to assert on the current_uri following a redirect.

session
|> visit("/dashboard")
|> assert_has(".error", "Requires login")
|> assert_uri("/login")
|> fill_form("#login-form", name: "Aragorn", email: "aragorn@dunedain.com")
|> click_button("Login")
|> assert_uri("/dashboard")

@germsvel Love this project

[sodapopcan]

Ah ya, that would probably be much better and pretty straightforward to implement.

EDIT: Though I think assert_path might be a better name since /dashboard is technically a URI fragment.

[germsvel]

@rgraff thanks for opening this issue and starting this discussion!

I've personally been of a mixed mind on asserting the current URL in tests in the past -- sometimes I think it's useful but sometimes I think it's a pointer towards the test wanting to assert something else. But I'm definitely open to changing my mind!

This is my current thought:

I typically try to keep my tests from the perspective of the user. So, whenever I want to assert what the current URL is, I ask myself: what are we really trying to assert?

Are we trying to assert that the user is in the expected page? If so, could we assert something that the user should see on that page? (not that users can't see the URL, but it's not really something they typically look at). Or is it that I (as the test writer) am unsure what page the test is landing on? (That points more towards improving the debugging experience.)

What do you think? Do you have good scenarios/use cases that we should consider?

[sodapopcan]

I'm likely just old but if it wasn't for newer browsers trying to pretend it doesn't exist I say that the URL is part of the user experience. It's more pervasive than some think. I've worked with very non-technical people who used the URL such as preferring to type in an order number into the address bar rather than use the provided form input.

On the flipside, it's really just enforcing a design standard. Changing code to no longer load based on a URL is not a refactor it's a code change. I could be proven wrong but it's pretty hard to accidentally start loading code from assigns instead params, so anyone who has decided to do this isn't going to care if the test breaks, they'll just delete the assertion. I guess it's possible to do it accidentally, though 🤔 I'm more trying to see the other side of the argument... I would personally be happy to have assert_path.

[rgraff]

@sodapopcan yeah, I like assert_path better too. 😄 Maybe there are some instances with multiple-tenancy apps that use subdomains a full uri? I'm not sure. 🤷‍♂️

@germsvel There are a number of use cases where you want to test from the perspective of the user.

test "a user is always returned to the page requested after logging in" do
  path = random_protected_path()
  session
  |> visit(path)
  |> assert_has(".error", "Requires login")
  |> assert_uri("/login")
  |> fill_form("#login-form", name: "Aragorn", email: "aragorn@dunedain.com")
  |> click_button("Login")
  |> assert_path(path) # The user has been given the requested content.
end

test "a report with complex filters is bookmarkable by the user" do
  session
  |> visit("/report")
  |> fill_form("#report-filter", customer: "Acme", sales_from: "2020-01-01", sales_to: "2020-12-31")
  |> click_button("View Report")
  |> assert_has(".report_row", "....")
  |> assert_path("/report", [customer: "Acme", sales_from: "2020-01-01", sales_to: "2020-12-31"]) # This report is now bookmarkable
end

test "a picture is shareable on social media" do
  session
  |> visit("/gallery")
  |> click_link("pic")
  |> assert_path("/gallery/pic")
  |> assert_has(".social-widget", "Share") # This widget will share the current uri
end

Because LiveView can change the content of the page without changing the uri/path, I think it's more important to be able to explicitly assert on the URI is some use cases (like above).

From a migration perspective (moving to PhoenixTest), it does help to have parity with the asserts in Phoenix.LiveViewTest.

I also don't think all tests need to be from the perspective of the user, sometimes you're testing for the side affects too (like that the social media post will link to the right uri when shared).

I hope this explanation helps.

[sodapopcan]

Maybe there are some instances with multiple-tenancy apps that use subdomains a full uri? I'm not sure. 🤷‍♂️

This is a good point. I actually wrote a toy multi-tenancy app with subdomains that I can't find atm and don't remember what I did, haha. But it could always be a :host option:

assert_path(~p"/some/path", host: "sub.domain.com")

Though against I'm spitballing without thinking it through too much.

I also don't think all tests need to be from the perspective of the user, sometimes you're testing for the side affects too (like that the social media post will link to the right uri when shared).

I think this still falls under what the user sees, no? I do quite well understand all about feature tests being from the user's perspective and maybe this falls a bit outside of what some people mean, but the browser does tell you where links go when you hover over them.

[sodapopcan]

Just quickly correcting myself here, :host would simply be the subdomain, not the 2LD (or TLD, for that matter).

[germsvel]

Thanks for the examples and discussion @rgraff and @sodapopcan!

I see why people might want an assert_path helper. I like that helper.

Would the API be like this?

# basic assertion
assert_path("/path/to/assert")

# with query params?
assert_path("/report", customer: "Acme", sales_from: "2020-01-01", sales_to: "2020-12-31") 

# do we want things like `:host`? 
assert_path(~p"/some/path", host: "sub.domain.com")

Do we want to support things like host: and query params? And if so, how do separate the two? The last two examples above conflict (in my mind). We could add query at the same level as host and that might fix the issue:

# with query 
assert_path("/report", query: %{customer: "Acme", sales_from: "2020-01-01", sales_to: "2020-12-31"}) 

# now host and query are at the same level
assert_path(~p"/some/path", host: "sub.domain.com")

What do you all think? Mind you, I haven't really thought of how we'd implement this yet. But if either of you has ideas and someone wants to take a stab at it, I'd love the help! 😄

[sodapopcan]

I did consider params although I've never asserted on them myself and I don't think there is precedent in LiveViewTest. But if it were to be included, I think your API is good.

I really think the host option should just be the subdomain like so:

|> assert_path(~p"/some/path", host: "sub.")

There is precedent in Phoenix for this syntax (see the third example in that section). Furthermore, I always set up my sub domain projects for local development to be domain agnostic. It allows people to follow their own host file conventions and I find it generally bad practice for an app to care about the 2LD+TLD.

Finally, I'm happy to take on the work here. I already have some momentum! But I'm also happy to let you do it @rgraff if you want to.

[rgraff]

I like the proposed syntax. I'm happy to do a PR but with my current work deadlines, it'll be a few weeks at least. @sodapopcan if you have bandwidth, please do!

[sodapopcan]

I very recently do have extra time now so I will put together a PR!

[rgraff]

@germsvel thank you. 🙇