Add assertion benchmarks #165
Conversation
Environment set also to test so that the testing webapp is available
Includes formatter change as the file is in a new bench folder.
3 examples (plain assert, tag, id+tag). Examples are run each one with a separate session in order to avoid any implicit optimization deriving from the reuse of the same session. Note that this cannot be avoided when optimizations kick in when calling assert_has/2-3 on the same session multiple times.
Returns {:error, :nosession}
|
EDIT: this was resolved. @germsvel not sure if you came across this issue. Using the standard LV test infra outside of test modules requires some ceremony due to the need to setup the When running this benchmark with The offending line is https://github.com/germsvel/phoenix_test/pull/165/files#diff-3e1e203eb1762597a06f32a45c52f756ae5c8e42b20bfd4f2d955436e9a92393R61: {:ok, view, html} = live(conn, "/page/index")I suspect I'm missing some setup code, but looking at a standard LV application I can't see any. The error seems to stem from https://github.com/phoenixframework/phoenix_live_view/blob/38be0a11eb8f509ac6995c5557d782327371820c/lib/phoenix_live_view/test/live_view_test.ex#L343, so unless you have any suggestion on how to fix this, I'll read the source from there and try to see what is going wrong. |
|
Never mind - it's just that I was using a static page path, I misread the router. Nothing to see here... |
|
We're getting somewhere. For the sake of moving forward, I wrapped the entire benchmark in a ExUnit test. This is needed because LiveView helpers do not work unless they're run inside an actual test: I'll think of a more elegant approach, but for now it will do. Here's the output of a benchmark run on my machine (Mac mini m4): I'll now finish writing the missing benchmarks. |
|
We got a first stab, here's some more results: |
|
@germsvel just wanted to ask if you need more info to move this forward (no rush)? I know I left one point open (running tests on CI) because I wanted to focus on "is this useful at all" first. Thanks! |
|
@cloud8421 this is incredibly helpful! Thank you so much! 🙏 I don't think you should do any more work, since I wouldn't imagine this is something we'd need to do in every CI run or anything like that (unless you have an idea for that? In which case, I'm all ears). But having this branch (and those initial benchmarks) will allow us to have an awesome starting point. Update: I honestly didn't envision this running in CI, but now that you mention it, is that something you think we could do? Could be very interesting knowing how benchmarks change with changes, and having this be auto-generated. Would love your thoughts there. |
|
@germsvel thank you! I think a weekly run against |
|
Elaborating further: the project moves at a speed where I think a weekly run is enough to surface issues, and if that happens one can easily bisect until the reason is clear. As a maintainer I think you have an option to manually unblock these tests for PRs, so that they don't run automatically (because I think they would burn capacity fairly quickly). |
Yeah, I think weekly would be fine. And I certainly don't want speed regressions to be a blocker -- sometimes we may have to make things slightly slower. But I would like to keep an eye on it to make sure we're not getting super slow.
Yep, happy to enable that. I guess my question is, do we include the CI work here? Or is that something for the future? We can merge this as-is, and leave CI for future work. I ran this locally, and it's really nice to have. |
|
I'd say if you're happy let's merge this and I'll tackle the CI bit separately. Thanks! |
|
Thanks so much @cloud8421! |
Closes #164
Add the first set of benchmarks for assertions.
PhoenixTestbased assertionsPhoenixTestbased assertions usingwithinCI will be addressed separately in another PR.