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] Comment Reply on Notes #8844

Merged
merged 6 commits into from
Dec 14, 2020

Conversation

noi5e
Copy link
Contributor

@noi5e noi5e commented Dec 14, 2020

Prerequisite: Merge #8809 First!

  • Add new fixtures for testing note comments:
    • node.yml
    • comment.yml
    • revision.yml
  • New test:
    • find first comment ID (the new comment fixture)
    • POST new comment with addComment
    • assert that comment text exists
test 'note: respond to existing comment' do
  visit nodes(:comment_note).path
  # find comment ID of the first comment on page
  parent_id = "#" + page.find('#comments-list').first('.comment')[:id]
  # comment ID format is id="c9834"
  # regex to find everything after the "c"
  parent_id_num = /c(\d+)/.match(parent_id)[1] 
  # parameters for addComment: addComment(comment text, submitURL, comment's parent ID)
  page.evaluate_script("addComment(\"I admire you\", '/comment/create/#{nodes(:comment_note).nid}', #{parent_id_num})")
  # check for comment text
  assert_selector("#{parent_id} .comment .comment-body p", text: 'I admire you')
end

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

@gitpod-io
Copy link

gitpod-io bot commented Dec 14, 2020

@noi5e noi5e added outreachy testing issues are usually for adding unit tests, integration tests or any other tests for a feature labels Dec 14, 2020
@jywarren
Copy link
Member

Once we merge #8809, and resolve any conflicts here, can you confirm that this passes locally so we can do a final review before merging (and bypassing Travis for now)? Thank you!

@jywarren
Copy link
Member

We can mark this with readytomerge as soon as we get conflicts resolved and local test passing confirmed. Thanks!

@noi5e
Copy link
Contributor Author

noi5e commented Dec 14, 2020

@jywarren Thanks, just resolved the conflicts and ran all tests locally again, looks good on my end (no additional/distinct failures)

@jywarren
Copy link
Member

Awesome, one more conflict resolution here, and we should be good to merge! Thank you! With all this happening, are you able to track the changes (i.e. check off items) in your main project issue as well? Just to keep us organized. Thanks @noi5e !!!

@jywarren
Copy link
Member

Ah i see each PR has a line and they're getting crossed off in #8775 - perfect!!

@codeclimate
Copy link

codeclimate bot commented Dec 14, 2020

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

View more on Code Climate.

@noi5e
Copy link
Contributor Author

noi5e commented Dec 14, 2020

Did another conflict resolve, so it should be ready!

I know it's getting a bit messy, I'll do a big organization and renaming of things after I have a decent set of tests.

@jywarren jywarren merged commit 6e4a175 into publiclab:main Dec 14, 2020
@jywarren
Copy link
Member

Awesome. Thanks again!!!! Probably best to run local tests one more time just to confirm after all the merging, deconflicting, etc.

Thanks a ton! I appreciate the extra time it took to get these organized given the Travis outage. 🙌

@noi5e noi5e deleted the test/note-comment-response branch December 14, 2020 22:48
@noi5e noi5e changed the title [Outreachy Comment Editor] New Test: Respond to Note Comment Testing: Comment Reply on Notes Dec 27, 2020
@noi5e noi5e changed the title Testing: Comment Reply on Notes [System Tests] Comment Reply on Notes Jan 23, 2021
manchere pushed a commit to manchere/plots2 that referenced this pull request Feb 13, 2021
…ab#8844)

* add note fixture 4 testing comments

* add comment 4 testing comment replies

* add new test: respond to note comments

* changed test name: 'respond to existing comment'
lagunasmel pushed a commit to lagunasmel/plots2 that referenced this pull request Mar 2, 2021
…ab#8844)

* add note fixture 4 testing comments

* add comment 4 testing comment replies

* add new test: respond to note comments

* changed test name: 'respond to existing comment'
reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this pull request Oct 16, 2021
…ab#8844)

* add note fixture 4 testing comments

* add comment 4 testing comment replies

* add new test: respond to note comments

* changed test name: 'respond to existing comment'
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this pull request Dec 28, 2021
…ab#8844)

* add note fixture 4 testing comments

* add comment 4 testing comment replies

* add new test: respond to note comments

* changed test name: 'respond to existing comment'
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.

2 participants