Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[System Tests] Post Replies with Multiple Comment Boxes Open #8881

Merged
merged 4 commits into from Jan 6, 2021

Conversation

noi5e
Copy link
Contributor

@noi5e noi5e commented Dec 22, 2020

I have another PR open at #8879, merge that one first!

  • Made new fixtures for question, wiki, note -- each with multiple comments on them.
    • I tried adding comments to the existing fixtures, but the assertions for earlier tests throw errors because Capybara expects to find just one element, but finds several.
    • It seemed a lot easier to just make new fixtures, rather than rewrite the old tests... For now.
  • Because of the new fixtures, I also had to alter the assertions in unit tests that count questions, wikis, notes. We ran into this problem before (I'll rewrite these someday!)

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

@noi5e noi5e added testing issues are usually for adding unit tests, integration tests or any other tests for a feature outreachy labels Dec 22, 2020
@gitpod-io
Copy link

gitpod-io bot commented Dec 22, 2020

@codeclimate
Copy link

codeclimate bot commented Dec 22, 2020

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

View more on Code Climate.

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8881   +/-   ##
=======================================
  Coverage        ?   81.85%           
=======================================
  Files           ?      100           
  Lines           ?     5935           
  Branches        ?        0           
=======================================
  Hits            ?     4858           
  Misses          ?     1077           
  Partials        ?        0           

@noi5e noi5e changed the title Test: Post Replies with Multiple Comment Boxes Open Testing: Post Replies with Multiple Comment Boxes Open Dec 27, 2020
@jywarren
Copy link
Member

jywarren commented Jan 5, 2021

It seemed a lot easier to just make new fixtures, rather than rewrite the old tests... For now.

Another way to do it is to add the comments within the test, using @node.add_comment(...) - esp if they're only going to be used in these tests. But, it's a stylistic or maintainability-related preference and your approach sounds great! This looks good and we can wait on #8879 before merging this but it looks super to me!!

@noi5e
Copy link
Contributor Author

noi5e commented Jan 6, 2021

Thank you @jywarren, I didn't know about @node.add_comment(...), will definitely look into that.

I'll make some time toward the end of the internship to clean up all the tests and fixtures I'm making, will probably use that if these fixtures don't get much use.

@noi5e
Copy link
Contributor Author

noi5e commented Jan 6, 2021

Oh yeah this is ready to merge too!

@jywarren jywarren merged commit 053e011 into publiclab:main Jan 6, 2021
@jywarren
Copy link
Member

jywarren commented Jan 6, 2021

Yay!!!

@noi5e noi5e deleted the test/multiple-comment-boxes branch January 6, 2021 04:39
@noi5e noi5e changed the title Testing: Post Replies with Multiple Comment Boxes Open [System Tests] Post Replies with Multiple Comment Boxes Open Jan 23, 2021
manchere pushed a commit to manchere/plots2 that referenced this pull request Feb 13, 2021
* add new wiki/note/question fixtures w/ multiple comments

* +1 assertions that count questions/wikis/notes

* new test: post replies w/ multiple comment boxes

* remove take_screenshot
lagunasmel pushed a commit to lagunasmel/plots2 that referenced this pull request Mar 2, 2021
* add new wiki/note/question fixtures w/ multiple comments

* +1 assertions that count questions/wikis/notes

* new test: post replies w/ multiple comment boxes

* remove take_screenshot
reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this pull request Oct 16, 2021
* add new wiki/note/question fixtures w/ multiple comments

* +1 assertions that count questions/wikis/notes

* new test: post replies w/ multiple comment boxes

* remove take_screenshot
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this pull request Dec 28, 2021
* add new wiki/note/question fixtures w/ multiple comments

* +1 assertions that count questions/wikis/notes

* new test: post replies w/ multiple comment boxes

* remove take_screenshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
outreachy testing issues are usually for adding unit tests, integration tests or any other tests for a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants