Add 'Mojolicious::Guides::Testing' tutorial. Resolves #1111 #1118

scottw · 2017-08-03T01:40:56Z

Summary

This is a testing tutorial meant to be included in the Mojolicious::Guides

Motivation

This PR is in response to #1111

References

Notes

It's a little weak in some areas (WebSockets, HTML/XML parsing come to mind) and I don't cover file uploads at all. I'm willing to continue work on it as needed.

scottw · 2017-08-03T01:50:54Z

I started with this as a section in the cookbook as originally described, but found that it got big really fast. I think having a cookbook section may still be valuable.

s1037989 · 2017-08-03T02:36:54Z

lib/Mojolicious/Guides/Testing.pod

+because assertions that belong to the same request are syntactically bound in
+the same method chain.
+
+Occsionally it makes sense to break up a test to perform more complex


Occasionally is misspelled here.

Emacs, you have failed in your mission.

s1037989 · 2017-08-03T02:42:51Z

lib/Mojolicious/Guides/Testing.pod

+  # Pull out the id from the response
+  my $newbee = $t->tx->res->json('/id');
+
+  # Make a new request with data from the previous response


This is great to share that you can do this. My only comment is that I've been told (my understanding may be way wrong) that tests shouldn't be built this way. I don't fully understand why. @jberger gave me the advice. If possible, I would love to have better explanations to how tests should be made (links to materials to read, etc). I know it's not exactly Mojolicious-specific, perhaps a reference to basic Testing advice would fit in the opening doc that points us to Learning Perl in 2.5 hours.
I don't understand good testing techniques, I just read the Test::Mojo docs and did what they allowed, they allowed me to do what's suggested here on L109, but @jberger suggested this wasn't a good way to design tests. Again, I may be misunderstanding. Any help in this regard would be wonderful, it would help users of Mojolicious to write really high quality apps that Mojolicious can be proud to be behind. It's not good for Mojolicious to have bad apps written (just look at the stigma associated with Perl from early on!)

If there's a better pattern for picking out data from one response and using it in a subsequent request, I'd love to see it and incorporate it here.

My point was, as I understand it, that one shouldn't pick out data from a response and use it in a subsequent request. I don't really understand why. Perhaps it was why I was picking data from a response and using it; surely there is a good reason to do so. Perhaps @jberger can help clear this up. Either way, I think it would be great to explain a recommendation in this regard. My reason for saying I think it would be good to explain this is that as you've done you explain that you can do this, and this leads me to go on to create tests that pick data from a response and use it in a subsequent request, but then I'm advised to not do that, I'm confused because the documentation says that I can... Hope that makes sense.

It does make sense. I agree that if these are, say, unit tests, it's probably not a good idea to start slinging state around like that because you're likely conflating model and view/controller concerns, but for an integration test it makes perfect sense, especially if your application is stateful in some way. I'd be reticent to label stateful tests as inherently wrong.

s1037989 · 2017-08-03T02:43:28Z

lib/Mojolicious/Guides/Testing.pod

+    ->json_is('/name' => 'Karl');
+
+The L<Test::Mojo> object is I<stateful>. As long as we haven't started a new
+transaction by invoking one of the C<*_ok> methods, the request and response


This is highly informative! I did not know this!! Thank you!!

I might take it further. Having access to the $t->tx means that you can test things that we don't have a convenience method for.

cmp_ok $t->tx->res->dom->at('epoch'), '>', 0, 'the current epoch value must certainly be before 1970!';

I think I made this clear at several other points later in the document… I didn't want to put too much "WOW" too early. Thoughts?

s1037989 · 2017-08-03T02:45:35Z

lib/Mojolicious/Guides/Testing.pod

+the L<Test::Mojo> constructor, L<Test::Mojo> will instantiate the class and
+start it, and cause it to listen on a random (unused) port number. Testing a
+Mojolicious application using L<Test::Mojo> will never conflict with running
+applications, including the application you're testing.


Great info!!

s1037989 · 2017-08-03T02:48:10Z

lib/Mojolicious/Guides/Testing.pod

+  # Normal route to controller
+  $r->get('/')->to('example#welcome');
+
+  # NEW: this route only exists when 'enable_test' is set in the configuration


Love this!! I've always wanted to have solid advice regarding this.

While this is true, I don't like to encourage having code that only supports tests in your application/production-code. In this case, before serving the first request you could add your /testing route

$t->app->routes->get('/testing')-> ...

Personally I think a better example is to specify some testing resource, like say a sqlite database or some external url to be used by the application. In your test config you can point it at some dummy or even mock source.

This is basically a "feature flag", though I didn't call it that. Feature flags allow you to soft-target an audience without doing a full release. Once it's proved out, you refactor the conditional and turn it into a release. Perhaps if I changed the word 'test' and 'testing' in the URL and flag to something like 'feature' would help clarify that? I didn't want to conflate release management with testing, only illustrate how nice it is that Test::Mojo lets you pass in config on the fly.

Perhaps I'll add another example with a database since both you and Stefan asked for that.

s1037989 · 2017-08-03T03:03:16Z

lib/Mojolicious/Guides/Testing.pod

+    ->status_is(200)
+    ->content_is('You are logged in');
+
+  # Sends the cookie from the previous transaction


Question: earlier you stated that as long as we haven't started a new transaction by invoking the *_ok methods then the request and response are available, so why are they available here on L523 when on L518 a new transaction should have started? What am I misunderstanding from your guide regarding this topic?

What you're seeing in this example is Test::Mojo using its Mojo::UserAgent object's cookie handling to keep state between requests.

s1037989 · 2017-08-03T03:06:07Z

lib/Mojolicious/Guides/Testing.pod

+level higher (which tells L<Test::Builder> how far up the call stack to look
+when something fails). Finally we use L<Test::More>'s C<is> method to compare
+the location header with the expected header value. We wrap that in the
+C<success> method, which propagates the object for method chaining.


Really happy you explained this!

s1037989 · 2017-08-03T03:07:11Z

lib/Mojolicious/Guides/Testing.pod

+
+  {"temperature":"23°C"}
+
+To test kind of web application, this we could do something like this for


To test this kind of web application, ~~this~~ we could do something like this for

s1037989 · 2017-08-03T03:13:04Z

lib/Mojolicious/Guides/Testing.pod

+requests to our deployed web application, and the test assertion (C<post_ok>)
+preserves compatibility with L<Test::Mojo>.
+
+=head1 MORE


One other thing that would be great to cover is how to handle testing of applications that utilize a database. For a bad example, let's say by hitting / the database counter is incremented by 1. So we get a 200 response but did it actually update the database counter? IIUC that should be unit tested into a Model::DBCounter test which is outside of the scope of Mojolicious, but I personally think some pointers or advice to some degree would be great for helping to ensure that authors of Mojolicious-based apps do as good of a job as possible.

I have given some MVC training using Mojolicious in the past that might be a good start for that sort of tutorial. I agree that it's out of scope for this, but would be valuable information nonetheless.

s1037989 · 2017-08-03T03:13:36Z

lib/Mojolicious/Guides/Testing.pod

+channel C<#mojo> on C<irc.perl.org>
+(L<chat now!|https://chat.mibbit.com/?channel=%23mojo&server=irc.perl.org>).
+
+=cut


WOW!!! Seriously, this guide is SOOO fantastic! You did a great job, thank you so much! I hope it gets approved and brought into core!!! :D

Thanks Stefan—I appreciate the feedback!

s1037989 · 2017-08-03T03:59:13Z

lib/Mojolicious/Guides/Testing.pod

@@ -646,7 +646,7 @@ Or:

  {"temperature":"23°C"}

-To test kind of web application, this we could do something like this for
+To test this kind of web application, this we could do something like this for


Also remove "this" from right after the , ... yes?

Yes, good catch. I must have been dozing when I wrote that.

kraih · 2017-08-03T08:15:22Z

This is a lot to review, yay! 😃

kraih · 2017-08-03T08:16:48Z

lib/Mojolicious/Guides/Testing.pod

+
+  $t->websocket_ok('/hello')
+    ->message_ok
+    ->json_message_like('/howdy' => qr/pard/i);


This should replace the cookbook recipe.

Do you mean I should replace this snippet with the cookbook recipe (which is a superior example imo) or vice versa? I didn't spend much time on WebSockets and would like a better section here. Can you clarify?

The cookbook recipe should be removed and this one improved.

jberger · 2017-08-03T08:23:31Z

lib/Mojolicious/Guides/Testing.pod

+
+A test file for a simple web application might look like:
+
+  use Test::Mojo;


You probably want use Mojo::Base -strict here or at least some strict/warnings enabled.

jberger · 2017-08-03T08:38:25Z

lib/Mojolicious/Guides/Testing.pod

+
+  $t->post_ok('/doorbell',
+              form => { action => "ring once" });
+  cmp_ok($t->tx->res->code, "<", 400, "status is good");


I'm not sure I love comparing the status code as <400. Perhaps some other example can be dreamt up to fill this space?

The problem is that Test::Mojo's assertions cover nearly everything needed under normal testing conditions, so it's hard to come up with meaningful out-of-bounds examples. I'm open to suggestions to illustrate what's possible while keeping it real.

Test $t->tx->res->message instead.

kraih · 2017-08-03T15:06:31Z

lib/Mojolicious/Guides/Testing.pod

+  # Still first transaction
+  $t->content_type_is('application/json');
+
+  # Second transaction; Test::Mojo tx object is reset


That comment sounds really bad to me, Test::Mojo tx object is not a thing.

kraih · 2017-08-03T15:07:43Z

lib/Mojolicious/Guides/Testing.pod

+is running and make requests to it. Once the tests have completed, the
+L<Mojolicious> application will be torn down.
+
+  my $t = Test::Mojo->new('Frogs');  ## runs on localhost:32114


...; ## runs on localhost:32114 is not really a comment style we use in Mojolicious.

kraih · 2017-08-07T22:11:01Z

lib/Mojolicious/Guides/Testing.pod

+introspected from L<Test::Mojo> through the application object. This enables
+us to get deep test coverage of L<Mojolicious>-based applications.
+
+=head1 ASSERTIONS


The new sections have no sentence describing them though.

I'm having a hard time coming up with reasons for me missing all these details that don't make me look stupid! :)

You're doing great, i'm having a hard time finding new things to nitpick about. 😉

On the upside, I can see why Mojolicious is so uniformly fantastic…

kraih · 2017-08-08T11:39:39Z

Regarding Test::Mojo extensions, we've decided to add a more broad mechanism to Mojo::Base with Role::Tiny. That means roles will look just like the ones for Test::Mojo::WithRoles.

package Test::Mojo::Role::Location;

use Role::Tiny;

sub location_is {
  my ($t, $value, $desc) = @_;
  $desc ||= "Location: $value";
  local $Test::Builder::Level = $Test::Builder::Level + 1;
  return $t->success(is($t->tx->res->headers->location, $value, $desc));
}

1;

But their application will be slightly different.

my $t = Test::Mojo->with_roles('Test::Mojo::Role::Location')->new('MyApp');
$t->get_ok('/')
  ->status_is(302)
  ->location_is('http://mojolicious.org')
  ->or(sub { diag 'Must have been Joel!' });

Mentioning it in case you want to prepare a section before we add the feature in the next few days. 😃

kraih · 2017-08-10T14:18:38Z

#1120 will get applied later today or tomorrow, so Role::Tiny is definitely the way to go for Test::Mojo extensions from now on.

scottw · 2017-08-10T15:21:23Z

I'll try to get something together for it.

kraih · 2017-08-10T18:57:44Z

lib/Mojolicious/Guides/Testing.pod

+But adding the C<Authorization> header for every request is tedious and likely
+to result in copy/paste errors given enough tests. We'd like to extend
+C<post_ok>'s behavior to build the header for us. With Mojolicious 7.41 (or
+later) and L<Role::Tiny> installed, we can compose this behavior with roles.


We don't mention Mojolicious versions in the docs.

kraih · 2017-08-10T18:58:25Z

lib/Mojolicious/Guides/Testing.pod

+      my $url = shift;
+      my $creds = shift; # username:password
+      my $headers = (ref $_[0] eq 'HASH' ? shift : {});
+      $headers->{Authorization} = "Basic " . b64_encode $creds, '';


local $Test::Builder::Level = $Test::Builder::Level + 1; is missing here.

kraih · 2017-08-10T18:58:48Z

lib/Mojolicious/Guides/Testing.pod

+  sub post_with_auth_ok {
+      my $t = shift;
+      my $url = shift;
+      my $creds = shift; # username:password


Unusual comment again.

And i don't see a reason not to write my ($t, $url, $creds) = (shift, shift, shift);.

kraih · 2017-08-10T18:59:36Z

lib/Mojolicious/Guides/Testing.pod

+
+First, we create a new package that will implement the role we want:
+
+  package Test::Mojo::Role::BasicAuth;


Think there should be an empty line here.

kraih · 2017-08-10T19:01:12Z

lib/Mojolicious/Guides/Testing.pod

+      my $t = shift;
+      my $url = shift;
+      my $creds = shift; # username:password
+      my $headers = (ref $_[0] eq 'HASH' ? shift : {});


This is a little clunky for a doc example, maybe assume the method always gets a hash with headers?

Well, actually it's not a very good example. Since we already support basic authentication through the URL.

Might as well do $url = Mojo::URL->new($url)->userinfo('foo:bar')->to_unsafe_string;.

I've removed this example.

kraih · 2017-08-10T19:05:15Z

lib/Mojolicious/Guides/Testing.pod

+
+  my $t = Test::Mojo->with_roles('Test::Mojo::Role::BasicAuth')->new('MyApp');
+
+  $t->post_with_auth_ok('/thermostat/heat', 'me:secretly',


Side note, $t->post_ok('http://me:secretly@/thermostat/heat', ...) should just work, sooo this actually requires more typing.

Yeah, I agree. When I realized that you don't get attributes with Role::Tiny, it makes this no better than the built-in, since you have to supply the credentials with every test. I'll pare this down in the next commit.

Technically, you could just use Mojo::Base to generate an attribute for you.

package Foo::Role; use Mojo::Base -base; use Role::Tiny; has whatever => 'Foo!'; sub do_stuff { say shift->whatever }

This just works. If that should be right in the first example... i'm not so sure about. 😃

I do like your current example though, it explains a lot of important topics very well.

kraih · 2017-08-10T20:50:35Z

lib/Mojolicious/Guides/Testing.pod

+We assign a default description value (C<$desc>), set the L<Test::Builder>
+C<Level> global variable one level higher (which tells L<Test::Builder> how
+far up the call stack to look when something fails), then we use
+L<Test::More>'s C<is> method to compare the location header with the expected


is is a function, not a method.

scottw · 2017-08-10T20:57:18Z

It's funny you bring this up because that's exactly what I was going to try when I had a little more time :) I think the existing `location_is` example is good enough to get people started. Any additional things you (or anyone else) would like to see in the tutorial?

scottw · 2017-08-10T20:58:26Z

Ok, maybe I'll put it back in as a second "advanced" example or something, that also blends attributes from Mojo::Base to illustrate how those two things can play together.

scottw · 2017-08-10T21:00:15Z

Good catch.

kraih · 2017-08-10T21:05:31Z

I think the guide is good now. Will give the community a little more time for proofreading and then merge tomorrow.

kraih · 2017-08-10T23:49:24Z

Looks like we might not be able to merge after all. The newly added role composition section is a blocker for me. The code examples are just too ugly. I'll step back again until the dust has settled.

This reverts commit 2d4595e.

scottw · 2017-08-11T00:07:09Z

reverted

kraih · 2017-08-11T08:20:51Z

Allright, lets consider the guide frozen now, so we can wrap it up. Just typo and formatting fixes from now on.

scottw · 2017-08-11T20:02:06Z

100% ok by me.

kraih · 2017-08-11T22:57:25Z

And merged. 🎉 🎂 🎈 http://mojolicious.org/perldoc/Mojolicious/Guides/Testing

Add 'Mojolicious::Guides::Testing' tutorial. Resolves #1111

9ccea47

remove FIXME note

36515ef

s1037989 reviewed Aug 3, 2017

View reviewed changes

fix typo

2a9193b

s1037989 reviewed Aug 3, 2017

View reviewed changes

Scott Wiersdorf added 2 commits August 2, 2017 22:05

and another

02e0f9a

fix typo

29fe426

kraih added docs needs feedback work in progress labels Aug 3, 2017

kraih reviewed Aug 3, 2017

View reviewed changes

jberger reviewed Aug 3, 2017

View reviewed changes

add strict to example; mention feature flag

9877bad

kraih reviewed Aug 3, 2017

View reviewed changes

Scott Wiersdorf added 3 commits August 3, 2017 10:32

clarify feature flag example

dbe67be

fix sloppy nomenclature

0adeab6

fix bad comment

329d8f0

mojolicious deleted a comment from scottw Aug 7, 2017

kraih reviewed Aug 7, 2017

View reviewed changes

fix missing section headers

815ae7b

kraih approved these changes Aug 8, 2017

View reviewed changes

use full header name

3de22bb

extend test class with roles

0d0dc9d

kraih reviewed Aug 10, 2017

View reviewed changes

remove Basic authentication example

78d250b

kraih reviewed Aug 10, 2017

View reviewed changes

Scott Wiersdorf added 2 commits August 10, 2017 15:19

method -> function

814fdb9

add example illustrating roles and attributes

2d4595e

Revert "add example illustrating roles and attributes"

192cf5e

This reverts commit 2d4595e.

kraih merged commit b574e2a into mojolicious:master Aug 11, 2017


		{"temperature":"23°C"}

		To test kind of web application, this we could do something like this for


		A test file for a simple web application might look like:

		use Test::Mojo;


		First, we create a new package that will implement the role we want:

		package Test::Mojo::Role::BasicAuth;


		my $t = Test::Mojo->with_roles('Test::Mojo::Role::BasicAuth')->new('MyApp');

		$t->post_with_auth_ok('/thermostat/heat', 'me:secretly',

Add 'Mojolicious::Guides::Testing' tutorial. Resolves #1111 #1118

Add 'Mojolicious::Guides::Testing' tutorial. Resolves #1111 #1118

Conversation

scottw commented Aug 3, 2017

Summary

Motivation

References

Notes

scottw commented Aug 3, 2017

s1037989 Aug 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s1037989 Aug 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraih commented Aug 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraih Aug 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraih Aug 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraih commented Aug 8, 2017 • edited

kraih commented Aug 10, 2017

scottw commented Aug 10, 2017 via email • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraih Aug 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraih Aug 10, 2017 • edited

Choose a reason for hiding this comment

scottw commented Aug 10, 2017 via email

scottw commented Aug 10, 2017 via email

scottw commented Aug 10, 2017 via email

kraih commented Aug 10, 2017

kraih commented Aug 10, 2017

scottw commented Aug 11, 2017 via email

kraih commented Aug 11, 2017

scottw commented Aug 11, 2017 via email

kraih commented Aug 11, 2017 • edited

s1037989 Aug 3, 2017 •

edited

s1037989 Aug 3, 2017 •

edited

kraih Aug 3, 2017 •

edited

kraih Aug 3, 2017 •

edited

kraih commented Aug 8, 2017 •

edited

scottw commented Aug 10, 2017 via email •

edited

kraih Aug 10, 2017 •

edited

kraih Aug 10, 2017 •

edited

kraih commented Aug 11, 2017 •

edited