Skip to content

[15 min fix] Fixed flaky E2E test for comments#13380

Merged
nickytonline merged 3 commits intomasterfrom
nickytonline/15-min-fix-e2e-flaky-test-for-comments
Apr 14, 2021
Merged

[15 min fix] Fixed flaky E2E test for comments#13380
nickytonline merged 3 commits intomasterfrom
nickytonline/15-min-fix-e2e-flaky-test-for-comments

Conversation

@nickytonline
Copy link
Contributor

@nickytonline nickytonline commented Apr 13, 2021

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Fixed flaky test

Description

Reenabled and fix a flaky E2E test for comments.

Related Tickets & Documents

#13187

QA Instructions, Screenshots, Recordings

N/A. Just reenabling a test.

UI accessibility concerns?

N/A. Just reenabling a test.

Added tests?

  • Yes
  • No, and this is why: please replace this line with details on why tests
    have not been included
  • I need help with writing tests

[Forem core team only] How will this change be communicated?

Will this PR introduce a change that impacts Forem members or creators, the
development process, or any of our internal teams? If so, please note how you
will share this change with the people who need to know about it.

  • I've updated the Developer Docs and/or
    Admin Guide, or
    Storybook (for Crayons components)
  • I've updated the README or added inline documentation
  • I will share this change in a Changelog
    or in a forem.dev post
  • I will share this change internally with the appropriate teams
  • I'm not sure how best to communicate this change and need help
  • This change does not need to be communicated, and this is why not: please
    replace this line with details on why this change doesn't need to be
    shared

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

Eminem saying "Shady's back"

@nickytonline nickytonline added area: tests issues related to tests area: comments labels Apr 13, 2021
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Apr 13, 2021
@nickytonline nickytonline requested review from a team, aitchiss and djuber April 13, 2021 21:55
@nickytonline nickytonline added the merge by any core This tag is a signal that you are okay with any core team member merging your code for you. label Apr 13, 2021
@nickytonline nickytonline changed the title Fixed flaky E2E test for comments [15 min fix] Fixed flaky E2E test for comments Apr 13, 2021
@nickytonline nickytonline requested a review from msarit April 13, 2021 22:51
@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Apr 14, 2021
Copy link
Contributor

@aitchiss aitchiss left a comment

Choose a reason for hiding this comment

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

This looks good to me - thanks for looking into it!

@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Apr 14, 2021

// TODO: Flaky spec
xit('should reply to a comment with user mention autocomplete', () => {
it('should reply to a comment with user mention autocomplete', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, that's all you had to say!

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Apr 14, 2021
@nickytonline
Copy link
Contributor Author

I ran this 20 times locally and all was good and I've rerun the build node 4 (front-end tests/E2E test) 3 times, so feeling pretty confident that this flaky test is squashed.

@nickytonline nickytonline merged commit fab904f into master Apr 14, 2021
@nickytonline nickytonline deleted the nickytonline/15-min-fix-e2e-flaky-test-for-comments branch April 14, 2021 15:08
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: tests issues related to tests merge by any core This tag is a signal that you are okay with any core team member merging your code for you. PR: merged bot applied label for PR's that are merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants