System Tests: Comments [Outreachy]

[noi5e]

@jywarren @cesswairimu @sagarpreet-chadha I could use a little guidance on how to write tests for async comment testing. I realized that I don't fully understand what async means in this context.

This planning Google Doc mentions it on page 14:

(Test for) Comment boxes created asynchronously (like, reply form on a just-posted comment) rather than on a freshly loaded page

That's talking about this kind of action (except for the weird email that opens in the tab, I think that only happens locally for some reason):

I guess this would actually be an asynchronous comment response. Would a good test for this basically be:

• navigate to page, and stay on it
• post a comment
• wait asynchronously for the DOM to update with the comment
• then post a reply to the comment you just posted(?)

Second Question:

• What's an example of posting an asynchronous comment (different from the comment response I posted above)? So far I know two different methods for posting a comment... Would the tests for either of these be considered async? If not what?

[jywarren]

Yes, this sounds like a good workflow. One thing that may simplify things (worth trying) is that Capybara, which runs our system tests, automatically waits a bit until it can find a CSS match for an assertion in a test. So, if you do the action and then do a CSS assertion, it'll wait until the action has completed and the page is finished loading/reloading to try the assertion. I think there's a timeout but as everything is local it should be a pretty fast response from the server in any case. I think we've even left some comments in the code of the system tests noting this technique!

…

On Wed, Dec 9, 2020 at 3:18 AM noi5e ***@***.***> wrote: @jywarren <https://github.com/jywarren> @cesswairimu <https://github.com/cesswairimu> @sagarpreet-chadha <https://github.com/sagarpreet-chadha> I could use a little guidance on how to write tests for async comment testing. I realized that I don't fully understand what *async* means in this context. This planning Google Doc <https://docs.google.com/document/d/1hOqa4tC0jqEuSaur1zKMtnxnknkARui3HTKqO3AUFMA/edit> mentions it on page 14: *(Test for)* Comment boxes created asynchronously (like, reply form on a just-posted comment) rather than on a freshly loaded page That's talking about this kind of action *(except for the weird email that opens in the tab, I think that only happens locally for some reason)* : [image: Untitled 5] <https://user-images.githubusercontent.com/4361605/101602225-a29b9800-39b2-11eb-9ecb-d214a5c5facd.gif> I guess this would actually be an *asynchronous comment response*. Would a good test for this basically be: - navigate to page, and stay on it - post a comment - wait asynchronously for the DOM to update with the comment - then post a reply to the comment you just posted(?) *Second Question:* - What's an example of *posting an asynchronous comment* (different from the comment *response* I posted above)? So far I know two <https://github.com/publiclab/plots2/blob/c069cfbec093ede01d8731d3b60b91b452b5f84e/app/assets/javascripts/comment.js#L62-L70> different methods <http://url> for posting a comment... Would the tests for either of these be considered async? If not what? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8810>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6JYCENEGHEIE7RBYYITST4XDXANCNFSM4UTD6JKA> .

[jywarren]

perhaps the 2 levels of testing here are:

1. the one you've proposed -- posting a comment and without a page refresh, replying to it
2. trying some other action upon a newly created (by AJAX/JS) comment form - like uploading an image - and ensuring that it appears in the correct box and not in any others

My thought is that it's good to keep in mind that we want to be confirming the "normal" behavior of the form and comment editor that was just created -- so using it to post a reply is just one way we can confirm this. What do you think?

except for the weird email that opens in the tab, I think that only happens locally for some reason

This btw is apparently some Ruby gem that intercepts mails locally and opens them in a browser tab, to make debugging easier without having to set up an email service. I forget what it's called though!

[noi5e]

@jywarren Thanks for the pointers earlier this week BTW. I'm working on a test right now for editing a freshly posted comment.

We already have one test for editing an existing comment. This test works manually via JQuery, selecting CSS elements and clicking on them:

      # Wait for comment to upload
      wait_for_ajax
      # Edit the comment
      page.execute_script <<-JS
        var comment = $(".comment")[1];
        var commentID = comment.id;
        var editCommentBtn = $(comment).find('.navbar-text #edit-comment-btn')
        // Toggle edit mode
        $(editCommentBtn).click()
        var commentTextarea = $('#' + commentID + 'text');
        $(commentTextarea).val('Updated comment.')
        var submitCommentBtn = $('#' + commentID + ' .control-group .btn-primary')[1];
        $(submitCommentBtn).click()
      JS
      message = find('.alert-success', match: :first).text
      assert_equal( "×\nComment updated.", message)

I thought I would go the other route by skipping the manual part, and instead doing a direct POST request to /comment/update:

      page.execute_script(<<~JS, comment_id_num)
        ((commentID) => {
          const requestBody = { body: "nyah nyah" }
          sendFormSubmissionAjax(requestBody, "/comment/update/" + commentID)
        })(arguments[0])
      JS
      message = find('.alert-success', match: :first).text
      assert_equal( "×\nComment updated.", message)
      assert_selector("#{comment_id} p", text: 'nyah nyah')

For some reason the test fails and I get the following error:

I can think of a few possibilities here:

• the POST request is improperly formed for some reason
• there's this block in comment_controller.rb, maybe it's not triggering the save and "Comment updated." flash:

  • plots2/app/controllers/comment_controller.rb

    Lines 93 to 95 in 8ea3e49

    	if @comment.save
    	flash[:notice] = 'Comment updated.'
    	redirect_to @path + '?_=' + Time.now.to_i.to_s

  • EDIT: I just did some console logs in this block, and it looks like the test does make it into this block, which means the comment is saved. Maybe the redirect doesn't happen for some reason? The test screenshot makes it seem this way (no "Comment updated" message, the comment text remains unedited).

SIDE NOTE: Edit Comment Triggers Page Reload?

Editing a comment triggers a page reload, but posting doesn't.

Questions:

• Any thoughts on why the test is failing?
• Where in the code is making the page reload on edit comment? Is it possible to rewrite this?

[noi5e]

Did more research on the edit comment flow, logged a lot of variables to the console. Still don't understand why I don't get a page & comment reload when I simulate a POST request to comment#update, instead of manually clicking on buttons.

It's also not that urgent. There is already one update comment test for wikis that is all manually submitted... Upon thinking about it some more, I think it makes sense that most of the system tests should be manual anyway. Because that's the point of a system test, right? So I shouldn't be trying to simulate AJAX submissions, instead I should be simulating button clicks.

I do still kind of want to know more about the edit comment flow I posted about above. But am focusing my energy elsewhere for the time being, particularly these system tests (in order of completion):

1. post fresh note/question/wiki, then comment on it
2. multiple comment boxes open, post/reply/upload image, and see if something breaks.
3. formatting crosswiring: bold and italics in the right place, even if multiple comment boxes open

I think I might start to see failing tests for the last two, which I hope will be a good transition toward focusing more energy on bug-fixing.

[jywarren]

Hi, @noi5e -- just a quick response here, sorry i didn't get to this on Friday. A few ideas here, and you may know better than I which is most helpful:

• I believe in some places one needs to follow redirects?
• To get the alert, we need to either refresh the page to get the Ruby-based flash version, or to generate it in JS (there are a couple ways). Is

  plots2/app/assets/javascripts/submit_form_ajax.js

  Line 4 in 876d0fc

  function sendFormSubmissionAjax(dataObj, submitTo, responseEl = "") {

  actually making an alert?

Were you able to try running these JS steps exactly in the JS console? That may be the easiest way to see if your test is running properly.

instead I should be simulating button clicks

Yes, i think that makes sense. Otherwise they're more like unit tests of our JS, which are also valuable but should be more narrowly designed. Let me see if I can trace exactly the sequence of events for this...

[jywarren]

OK, with regard to the flash[:notice] style alerts, these only get rendered when the page is refreshed - it persists the message to the next page render -- so /after/ the render statement, or after a redirect statement.

I don't see a way that sendFormSubmissionAjax is set up to make a message... hmm. Aha! Ok, so it's triggering an event called ajax:success upon completion; that activates this segment of code:

plots2/app/assets/javascripts/comment.js

Lines 5 to 13 in 484bf69

	$(this).addClass('bound-success').bind('ajax:success', function(e, data, status, xhr){
	$(this).find('#text-input').prop('disabled',false);
	$(this).find('#text-input').val('');
	$('#comments-container').append(xhr.responseText);
	$(this).find(".btn-primary").button('reset');
	$(this).find('#preview').hide();
	$(this).find('#text-input').show();
	$(this).find('#preview-btn').button('toggle');
	});

Does that make sense? As to why the returned message is not activating that, let's first confirm that it happens when done manually in the console. If you do it you can also watch the Rails console at the same time to confirm that the POST request succeeds with a 200 success status.

If it succeeds locally/manually, then the problem may be that we need to wait for the returned AJAX to trigger that event and insert the message /before/ asserting the message content, so there may be a kind of wait_for_ajax type method we do to insert some wait time. See how that works here!

plots2/test/system/rich_text_editor_test.rb

Lines 35 to 39 in 876d0fc

	# Upload the image
	drop_in_dropzone("#{Rails.root.to_s}/public/images/pl.png", '.wk-container-drop')
	# Wait for image upload to finish
	wait_for_ajax

I think the actual code behind wait_for_ajax is here;

plots2/test/application_system_test_case.rb

Line 52 in 876d0fc

def wait_for_ajax

Hopefully that helps get this test running! Please reach out if you get stuck but i'll also be online all tomorrow as usual so we can pair up on this if it's helpful too.

Thanks @noi5e for sticking with this one! These are challenging tests and somewhat tangled code; you can see why we may want to at least insert more comments explaining how this works, and at best, perhaps refine the code structure some!

[noi5e]

@jywarren I appreciate those clarifications! That's a great tip to debug JS in the browser console. I didn't even think about that but of course it makes sense.

Even though I know to focus more on manual testing, I'll look at this to get a better general understanding of mechanics... I'm sure I'll get more insight. Thanks again.

[noi5e]

Okay, finally have some insight, making AJAX requests in the browser console was indeed super-helpful.

Here's what my simulated AJAX request looks like when it's received by the Rails console:

  Started GET "/comment/update/13?body=nyah+nyah&_=1608601205240" for 127.0.0.1 at 2020-12-21 17:42:01 -0800
  Processing by CommentController#update as */*
  Parameters: {"body"=>"nyah nyah", "_"=>"1608601205240", "id"=>"13"}

Here's what a typical AJAX request looks like when received via the edit comment form, as normal:

  Started POST "/comment/update/13" for 127.0.0.1 at 2020-12-21 17:51:29 -0800
  Processing by CommentController#update as HTML
  Parameters: {"authenticity_token"=>"nbOcOGXMKihOckFT5QOxDrWlHF2HEM4ONsoC9D64Qwb/CsO3BpXo5MueR5nTiu42zbVc26IYAmZ1m41Fc8v+wQ==", "image"=>{"photo"=>""}, "body"=>"nyah nyah!!!", "id"=>"13"}

🤦 GET vs POST! Luckily config/routes.rb can accommodate both GET and POST requests to /comment/update, and routes them toward the same comment#update action.

That's not all though, turns out that the edit form has an authenticity token embedded in it:

plots2/app/views/comments/_edit.html.erb

Line 5 in 6505f53

<input type="hidden" name="authenticity_token" value="<%= form_authenticity_token %>">

I'm not sure what this authenticity token does per se, it doesn't seem to be required to update comment content, since the comment updates just fine without it-- Is this maybe a security issue? comment#update does check for current.uid FYI.

The most interesting thing I found was this, which is the XHR responseText:

"Turbolinks.clearCache()\nTurbolinks.visit(\"http://localhost:3000/notes/user/12-08-2020/can-i-ask-a-question?_=1608604257\", {\"action\":\"replace\"})"

This looks like some under-the-hood Rails stuff which is the piece that actually does the redirect?

Just posting this here as a big old FYI. It's pretty interesting, but just confirms my decision to write all the tests manually from now on. 😅

[jywarren]

Ah! The token is required for Rails POST routes, i believe, perhaps especially ones sent by JavaScript... i forget exactly how it's tracked but the idea is to prevent random JS scripts from sending such requests outside the scope of a page where they're intended. So perhaps we just aren't triggering any security in the way we're using it right now?

Oof, it looks like that's left over from our attempt to use Turbolinks to accelerate page loads, which worked, but broke our Google Analytics scripts, so we turned it off; #2132 is an entry point here. If this is causing a failure, we may need to turn that response off -- i think what it may have been doing is initiating a Turbolinks-style redirect and ensuring it got refreshed with a new page, but we don't currently use Turbolinks. Gosh, Turbolinks is a fascinating system but it touches so many parts of a platform that it seems quite brittle!

[jywarren]

I think we're actually pretty good here, what do you think @noi5e? What other system tests are you planning to add, if any? Thank you!

[noi5e]

@jywarren Yeah, I agree. I think I have enough of an idea of how to proceed from here. Will just be adding system tests for bugs as they appear. Closing this issue for now at least.