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

[MM-27209] - Add spec for archived channels part 03 #7209

Merged
merged 6 commits into from
Jan 14, 2021
Merged

Conversation

nevyangelova
Copy link
Contributor

Summary

This PR adds 7 cases (for web) from the archived channels part 03 epic

Ticket Link

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

e2e/cypress/integration/channel/archived_channels_spec.js Outdated Show resolved Hide resolved
cy.apiAdminLogin();
cy.apiUpdateConfig({
ServiceSettings: {
ExperimentalChannelSidebarOrganization: 'default_off',
Copy link
Member

Choose a reason for hiding this comment

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

In hindsight, the more I see us doing this the more I wonder if we should just change the test procedure to use the new sidebar instead. What do you think/how hard is that to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ticket has tests for both new and old sidebar, and for example wants to test old sidebar specific features like channel grouping 🤷‍♀️ If we are getting rid of the old sidebar soon then these test cases will be redundant then too but for now I guess we are keeping them?

Copy link
Member

Choose a reason for hiding this comment

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

That's true, though I'm wondering what the value is in writing these tests for the old sidebar. Especially considering that the main purpose for the push in writing these tests is for cloud, which will never be using the old sidebar. Let's chat about this in triage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

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.

@nevyangelova Could you please run the tests and confirm if all are passing on you. Most are failing on me.
Screen Shot 2020-12-17 at 10 32 46 PM
Also after system config change, I suggests to use a regular user for testing (instead of sysadmin) as much as possible so we can ensure that it starts with the same expected user setting, and will not be affected by other test's change.

@nevyangelova
Copy link
Contributor Author

nevyangelova commented Dec 18, 2020

@saturninoabril @devinbinnie updated all tests to use the new sidebar. Removed Admin login and left it only for creating private channels. All tests are passing for me. Updated the test cases :) Removed MM-T1698 as an old sidebar feature test only.

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 @nevyangelova! Please see comments.

// # More channels list should contain the archived channel
cy.get('#moreChannelsList').should('contain', archivedChannel.display_name);
});
});
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting failing tests here, probably due to sysadmin's user preference. If possible, after config or other requests, please use regular user instead of sysadmin. Also, does the config change at MM-T1696 applicable to the succeeding tests. If yes, then I suggest to move all new tests to new spec file and have the config change in the before hook.
Screen Shot 2020-12-19 at 3 56 02 AM
Screen Shot 2020-12-19 at 3 56 39 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saturninoabril

Also, does the config change at MM-T1696 applicable to the succeeding tests. If yes, then I suggest to move all new tests to new spec file and have the config change in the before hook.

No, we only need it for that one test case to confirm that when archived channels feature is disabled users cannot viewthem.

@nevyangelova
Copy link
Contributor Author

@saturninoabril fixed, removed all admin user apart from MM-T1696 since that needs to change the config for the test, and I moved it below the other test cases because of the different config settings.

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 Nevy! LGTM, tested and all passed.

@saturninoabril saturninoabril added the 4: Reviews Complete All reviewers have approved the pull request label Jan 14, 2021
@saturninoabril
Copy link
Member

/update-branch

@saturninoabril saturninoabril added the AutoMerge used by Mattermod to merge PR automatically label Jan 14, 2021
@mattermod
Copy link
Contributor

Will try to auto merge this PR once all tests and checks are passing. This might take up to an hour.

@mattermod
Copy link
Contributor

Trying to auto merge this PR.

@mattermod mattermod merged commit 1f00961 into master Jan 14, 2021
@mattermod
Copy link
Contributor

Pull Request successfully merged
SHA: 1f00961

@mattermod mattermod deleted the MM-27209 branch January 14, 2021 10:45
@mattermod mattermod removed the AutoMerge used by Mattermod to merge PR automatically label Jan 14, 2021
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 14, 2021
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
Development

Successfully merging this pull request may close these issues.

5 participants