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

MM-27802 Cypress test for MM-T40 (1.0) Plugin remains enabled when upgraded #6294

Merged
merged 7 commits into from
Aug 28, 2020

Conversation

jfrerich
Copy link
Contributor

Summary

Cypress test for MM-T40 Plugin remains enabled when upgraded

Test Case: MM-T40

Additional Notes

The code for this test case has some similarities to the previous test that exists in this file. The other tests uses the draw plugin and I chose to leave it as is and add the new test case for the following reasons.

  • keep the test attached to the specific Jira test
  • eliminate some portion of the previous test being polluted by additional code
  • Use of additional plugin (as specified by the Jira task)

Ticket Link

https://mattermost.atlassian.net/browse/MM-27802

@jfrerich jfrerich added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Aug 27, 2020
Copy link
Contributor

@prapti prapti left a comment

Choose a reason for hiding this comment

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

Thank you so much @jfrerich for this test! The test is great! It also passes without fail but when the plugin is copied in the 'fixture' folder. So I've requested to add a comment to download and copy it in the required folder.
However, the test should go into a different folder from the one you've added to. I apologize for the confusion and I can see why that is. But this one is slightly different from the system console test. I think the rest of the plugin overwrite, enable, disable test etc. are in the Plugins folder in TM4J. Would you mind adding this test to the "Plugins" folder in cypress? It currently has a 'marketplace' subfolder, but I think it's fine to add a new spec file parallel to the marketplace subfolder.
Sorry about the long story, but please let me know if there is any confusion! (The other plugin tests in TM4J are missing from Cypress because I think they're automated in Selenium - the legacy automated test env).
Thank you Jason!

@@ -108,6 +110,64 @@ describe('Draw Plugin - Upload', () => {
cy.findByText(/Installed Plugins/).scrollIntoView().should('be.visible');
cy.findByTestId('com.mattermost.draw-plugin').should('not.exist');
});

it('M27802-MT40-Plugin remains enabled when upgraded', () => {
// * upload Demo plugin from the browser
Copy link
Contributor

Choose a reason for hiding this comment

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

The test as written here will require the demo plugin to be copied into the 'fixture' folder inside cypress, instead of it getting downloaded from the browser. That will actually be fine as well; however, to note that please add a comment above this test to download both the plugin versions from https://github.com/mattermost/mattermost-plugin-demo/releases (or the direct link to the release bundles within that link). I will also update the test case to reflect that. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the notes to the top of the test spec. If you prefer it directly above the it() block, I can move it. If you need any additional changes, please let me know!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jfrerich! I'm good with the notes location.
Hello @prapti, there are 2 steps in TM4J but maybe it's good to combine into one. I'll defer to you since you'll be updating the ticket. We just need to make sure that the number of steps should match between Cypress and TM4J.

@jfrerich
Copy link
Contributor Author

@thanks for the extremely detailed response. I am modifying the PR now!

@jfrerich jfrerich requested a review from prapti August 28, 2020 02:53
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.

Thanks @jfrerich! Overall looks good, tested and passed, except for few suggestions.

e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
@jfrerich
Copy link
Contributor Author

Thanks for the feedback, @saturninoabril! I have addressed your PR change requests.

I noticed in the cypress desktop I'm using version 4.12.1 but have an option to update. The version is currently set by our flow. Should I ignore the update notification?

image

@saturninoabril
Copy link
Member

Please ignore the updates. The next is major version 5.0 and there might be some breaking change in our tests. Also, others are seeing issues with 4.12.1 and so there's a PR to downgrade to 4.8.0 - #6304

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.

Thanks @jfrerich for updating. Last comments and I'm all good.

e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
e2e/cypress/integration/plugins/upgrade_spec.js Outdated Show resolved Hide resolved
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.

@jfrerich Thanks! Tested and passed.

Copy link
Contributor

@prapti prapti left a comment

Choose a reason for hiding this comment

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

Looks great and test passed! Thank you again for working on this @jfrerich!

@prapti prapti added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Aug 28, 2020
@prapti prapti merged commit 9f0520e into master Aug 28, 2020
@prapti prapti deleted the MM-27802_e2e-plugin-remains-enabled-on-upgrade branch August 28, 2020 18:36
@amyblais amyblais added the Changelog/Not Needed Does not require a changelog entry label Sep 6, 2020
@amyblais amyblais added the Docs/Not Needed Does not require documentation label Sep 6, 2020
jfrerich added a commit that referenced this pull request Oct 23, 2020
…graded (#6294)

* test for plugin enabled after second upload

* remove new test and revert to original.  test was added to incorrect
location

* add new plugin upgrade test

* update nits

* remove v.0.2.0 demo plugin after test finishes

* pass in filename to attachFile fuction as 'filename' parameter

* remove the plugin with inside the "after" hook
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
4 participants