Skip to content
This repository was archived by the owner on Mar 13, 2024. It is now read-only.

MM-15560 - E2E Fix: update markdown html fixtures #2792

Merged
merged 4 commits into from
May 21, 2019

Conversation

thekiiingbob
Copy link
Contributor

Summary

  • Updated fixtures for the expected HTML output of markdown for the markdown tests.

Ticket Link

JIRA

To run just these tests run npm run cypress:run -- --spec "cypress/integration/markdown/**/*.js" --reporter "spec" --browser "chrome"

@thekiiingbob thekiiingbob changed the title update markdown html fixtures MM-15560 - E2E Fix: update markdown html fixtures May 14, 2019
Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Could you please check again? I feel like it's some kind of a valid bug.

@thekiiingbob
Copy link
Contributor Author

@saturninoabril Should be good now, image proxy for sure impacts this test, so making sure we have it enabled for this test.

@thekiiingbob
Copy link
Contributor Author

@saturninoabril @migbot Latest changes pushed up. Using cy.route to wait for request to get posts completes before submitting any additional posts.

This should fix an issue @migbot was having.

Right now there are several tests that do the win.fetch = null thing (which causes requests that are fetch to fallback to XHR), which is not entirely ideal, but there is a long awaited open PR to add full network stubbing support - cypress-io/cypress#4176 , which is scheduled after Cypress's 3.3.0 release.

@hanzei hanzei added the 2: Dev Review Requires review by a core commiter label May 17, 2019
@saturninoabril
Copy link
Member

@thekiiingbob I've got test failures on "Markdown - in-line images 3 and 4" due to the pending post ID returned from getLastPostId (see screenshot). I think the previous implementation of "WithRetry" is better suited than the one we have now with hard limit timeout of [30 seconds].(

cy.get('#postListContent #postContent', {timeout: 30000}).last().parent().as('_lastPost');
)
Screen Shot 2019-05-17 at 5 49 50 PM
Screen Shot 2019-05-17 at 5 49 08 PM

@saturninoabril saturninoabril self-requested a review May 17, 2019 09:57
@thekiiingbob
Copy link
Contributor Author

thekiiingbob commented May 20, 2019

@saturninoabril I tested this latest code with caching disabled and under the Fast 3G network conditions, and the markdown tests pass for me. Give it a whirl under your environment and let me know if you are having issues. This should handle all the waiting we need for posts.

Note that the actual implementation really didn't change, but things are passing for me. Image 3 & 4 are definitely the big offenders though. They take the longest to wait for.

@saturninoabril
Copy link
Member

Sure, will give it a test. One thing though, the very long timeout, especially 150s seems a flaw to me. Do we have other choice?

@thekiiingbob
Copy link
Contributor Author

@saturninoabril Added timeouts constants file and started using it. Lemme know if the names of the consants is sufficient and clear enough, or if you had something else in mind. Image related markdown tests are now run after, with more intense timeout requirements added.

@thekiiingbob
Copy link
Contributor Author

Also, I had to update the expected HTML for the image tests. It seems that the alt attribute is now before the other attributes. If this changes again soon, I'd want to alter how we are doing these tests.

Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Works for me; all tests passed 💯

@migbot
Copy link
Contributor

migbot commented May 21, 2019

All tests passed as well.

@migbot migbot added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels May 21, 2019
@migbot migbot merged commit 613b919 into mattermost:master May 21, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels May 21, 2019
composednitin pushed a commit to composednitin/mattermost-webapp that referenced this pull request May 22, 2019
* update image proxy setting in test to make sure URLs match

* start cy server and wait for posts request to complete

* improve performance of waiting for new posts under bad network conditions

* integrate timeout constants, updated expected HTML
@thekiiingbob thekiiingbob deleted the MM-15560 branch June 28, 2019 15:18
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants