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

Add Tests for React Post Comment, Post Reply #9665

Merged
merged 1 commit into from
May 30, 2021

Conversation

noi5e
Copy link
Contributor

@noi5e noi5e commented May 24, 2021

See #9365

System tests for:

  1. Posting a comment in React
  2. Posting a reply to another comment in React

I made a new block in comment_test.rb to test run tests for basic comment CRUD functionality against both React notes, and then Rails notes. I reorganized the comment system tests a bit, and left some notes to explain the flow.


(This issue 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 JavaScript outreachy react labels May 24, 2021
@noi5e noi5e requested a review from jywarren May 24, 2021 21:26
@gitpod-io
Copy link

gitpod-io bot commented May 24, 2021

@@ -60,26 +112,6 @@ def get_path(page_type, path)
assert_selector("#{'#c' + parent_id_num + 'show'} div div div p", text: comment_response_text)
end

test "#{page_type_string}: manual comment and reply to comment" do
visit get_path(page_type, nodes(node_name).path)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted this test.

@codecov
Copy link

codecov bot commented May 24, 2021

Codecov Report

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

❗ Current head 870273c differs from pull request most recent head b17fcc3. Consider uploading reports for the commit b17fcc3 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9665   +/-   ##
=======================================
  Coverage        ?   81.89%           
=======================================
  Files           ?       98           
  Lines           ?     5928           
  Branches        ?        0           
=======================================
  Hits            ?     4855           
  Misses          ?     1073           
  Partials        ?        0           

@noi5e noi5e changed the title Add Tests for React Post Comment, Post Reply #9365 Add Tests for React Post Comment, Post Reply May 24, 2021
@jywarren
Copy link
Member

Looks like a unit test at Progress: |======================================================== FAIL NodeTest#test_should_find_all_questions (19.97s)

https://github.com/publiclab/plots2/pull/9665/checks?check_run_id=2659898221#step:5:36

This seems unrelated. It could be an ordering issue. I'll try restarting it!

@jywarren
Copy link
Member

Yes, nid 8 was still there but in a different order in the returned array. This may mean we need to add a sort to that test either in the main codebase or in the tests. We could try to resolve this here or in a distinct PR!

# create (posting comments & replies)
# update (editing comments)
# delete
[true, false].each do |is_testing_react|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice code syntax!!

@noi5e
Copy link
Contributor Author

noi5e commented May 25, 2021

Yes, nid 8 was still there but in a different order in the returned array. This may mean we need to add a sort to that test either in the main codebase or in the tests. We could try to resolve this here or in a distinct PR!

@jywarren, There's an issue open for the failed test at #9662. I looked into it further, I think it has something to do with array sorting. Check out this comment here.

@jywarren
Copy link
Member

This should work now that @cesswairimu merged a fix for the upstream issue! Thanks!!!

@jywarren jywarren force-pushed the react-comments-system-tests branch from efb3d34 to b17fcc3 Compare May 30, 2021 00:40
@jywarren
Copy link
Member

Rebased!!!

@codeclimate
Copy link

codeclimate bot commented May 30, 2021

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

View more on Code Climate.

@jywarren jywarren merged commit 7fe537a into publiclab:main May 30, 2021
@jywarren
Copy link
Member

Awesome! Thanks @noi5e !!!

@noi5e noi5e deleted the react-comments-system-tests branch May 31, 2021 04:27
@noi5e
Copy link
Contributor Author

noi5e commented May 31, 2021

@jywarren Thanks for the rebase!

reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this pull request Oct 16, 2021
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this pull request Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScript outreachy react 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