-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Conversation
@@ -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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted this test.
Codecov Report
@@ Coverage Diff @@
## main #9665 +/- ##
=======================================
Coverage ? 81.89%
=======================================
Files ? 98
Lines ? 5928
Branches ? 0
=======================================
Hits ? 4855
Misses ? 1073
Partials ? 0 |
Looks like a unit test at 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! |
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| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice code syntax!!
@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. |
This should work now that @cesswairimu merged a fix for the upstream issue! Thanks!!! |
efb3d34
to
b17fcc3
Compare
Rebased!!! |
Code Climate has analyzed commit b17fcc3 and detected 0 issues on this pull request. View more on Code Climate. |
Awesome! Thanks @noi5e !!! |
@jywarren Thanks for the rebase! |
See #9365
System tests for:
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)