[System Tests] Increase System Tests Speed by Decreasing Coverage

[noi5e]

Currently, all the comment system tests are tested on the three main page types:

• Research Notes
• Questions
• Wikis

In the past, this may have been necessary because the Comment Editor did sometimes behave differently, particularly on wiki pages.

However, with recent refactoring of editor.js, which I explained in this comment, I think we can be confident that comments will behave pretty similarly across all three page types. After all, they all derive from the notes/comments partial.

So I'm reducing test coverage and just running some system tests on only research notes, and other tests on all three (wikis, notes, and questions). This way, we can keep the multi-page testing block just in case we want to test on all 3 in the future.

---

(This issue is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)

[gitpod-io]

[noi5e]

This is how the tests were run on all three page types:

plots2/test/system/comment_test.rb

Lines 23 to 33 in f82a11c

	# page_types are wiki, research note, question:
	page_types.each do |page_type, node_name|
	page_type_string = page_type.to_s
	comment_text = 'woot woot'
	comment_response_text = 'wooly woot'
	test "#{page_type_string}: addComment(comment_text)" do
	visit get_path(page_type, nodes(node_name).path)
	page.evaluate_script("addComment('#{comment_text}')")
	assert_selector('#comments-list .comment-body p', text: comment_text)
	end

page_types is defined in test_helper.rb here:

plots2/test/test_helper.rb

Lines 43 to 50 in f82a11c

	# used in comment_test.rb
	def page_types
	{
	:note => :comment_note,
	:question => :comment_question,
	:wiki => :wiki_page
	}
	end

[noi5e]

This block just runs tests on research notes (most tests will now live in this block).

[noi5e]

I deleted this test (which I wrote early on in the internship), because it doesn't seem so necessary now.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@f82a11c). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #9045   +/-   ##
=======================================
  Coverage        ?   82.05%           
=======================================
  Files           ?      100           
  Lines           ?     5939           
  Branches        ?        0           
=======================================
  Hits            ?     4873           
  Misses          ?     1066           
  Partials        ?        0           

[noi5e]

In this block, tests are run on all three page types. Keeping the cross-wiring and image upload tests here for now, but perhaps they can be moved out soon.

[jywarren]

Yes, 16m is pretty good. Are there other places we could obviously speed things up? For example, i wonder if caching dependencies would make a significant difference; i wasn't sure the Yarn deps were getting correctly cached...

[noi5e]

I'll definitely do another re-assessment after the internship is over... I think if the editor code is overhauled, maybe we really won't need to do so many cross-wiring tests?

I don't know much about caching dependencies, do you have a link to read up on that?

[codeclimate]

Code Climate has analyzed commit d95e62e and detected 0 issues on this pull request.

View more on Code Climate.

[noi5e]

System tests passed in 16 minutes... My last two PRs had system tests passing in 25 minutes, but also 19 minutes. Seems like a slight improvement in speed!

Ready to merge with review!

[jywarren]

I think this looks great. I really appreciate that you're at a point where you can remove tests due to refactors or better understanding of the code. Nice!!