-
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
[System Tests] Consolidate Comment Tests for Wikis, Questions, and Notes #8860
[System Tests] Consolidate Comment Tests for Wikis, Questions, and Notes #8860
Conversation
Rebased to match the new and improved master! |
Hmm, it seems stalled? I'll try restarting it. Thanks! |
Hmm... That's odd. If it doesn't start soon, could you try a manual rebase with |
OK, we modified the trigger to be for |
Codecov Report
@@ Coverage Diff @@
## main #8860 +/- ##
=======================================
Coverage ? 81.86%
=======================================
Files ? 100
Lines ? 5938
Branches ? 0
=======================================
Hits ? 4861
Misses ? 1077
Partials ? 0 |
Code Climate has analyzed commit a2bd219 and detected 0 issues on this pull request. View more on Code Climate. |
I think this is unrelated to our code, but it's probably worth opening a new issue for it and trying to search for this issue in relation to github actions, or maybe in the mysql github action:
in the meantime i restarted! |
Looks great!! |
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.
Looks awesome. I'll merge it now!!!
|
||
comment_response_text = 'wooly woot' | ||
|
||
test "#{page_type_string}: addComment(comment_text)" do |
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.
huh cool! I didn't know you could nest test
blocks under a loop, but that makes sense as long as the names are unique! Great work!
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.
Thanks! 🎈 Creative solution, but it seems to work!
Awesome! So great to see everything pass here! Thanks @noi5e !!!! 👍 🎉 |
…8860) * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names
…8860) * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names
…8860) * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names
…8860) * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names * change :question4 to :comment_question * consolidate all comment tests * add helper function to retrieve fixture names
Instead of continuing to write distinct comment tests for wikis, questions, notes, etc. I decided to consolidate them.
The basic format of comment_test.rb is now:
NOTES:
(This PR is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)