Cypress/E2E: Fix bot accounts disabled section verification #6894
Conversation
There was a problem hiding this comment.
Thanks @josephbaylon! LGTM, except for one failing test on me.
| @@ -133,7 +133,7 @@ describe('Managing bot accounts', () => { | |||
| cy.visit('/login'); | |||
|
|
|||
| // # Enter bot name in the email field | |||
| cy.findByPlaceholderText('Email, Username or AD/LDAP Username', {timeout: TIMEOUTS.ONE_MIN}).clear().type(botName); | |||
| cy.findByPlaceholderText('Email or Username', {timeout: TIMEOUTS.ONE_MIN}).clear().type(botName); | |||
There was a problem hiding this comment.
This failed on me since LDAP is enabled at partial_default_config.json.
There was a problem hiding this comment.
Good to have: cy.apiAdminLogin() at L148 should move to beforeEach so that failed test here will not cause to fail other succeeding tests.
There was a problem hiding this comment.
- I removed the extra cy.apiAdminLogin at L148 since it's already present in beforeEach
- I ran the tests without license since it's not enterprise. Can you try that locally and see if you still get the LDAP issue?
There was a problem hiding this comment.
I see but I suggest to make this spec pass both for EE and TE since we can't make this TE only.
There was a problem hiding this comment.
I just moved that particular test case to enterprise without having to add additional test case in tm4j
There was a problem hiding this comment.
Thanks! Now I'm thinking about the conditional test that Rohitesh mentioned before, and something like this might make sense in doing so. But I'm good with the latest change.
There was a problem hiding this comment.
Well the thing is that case is about bot account not being able to login. The placeholder having different value for EE and TE is outside that case. There should be a new case just testing the placeholder values
There was a problem hiding this comment.
I understand, my only concern is the test should be able to run both for TE and EE. With the latest change, we'll not be able to run in TE since it's marked as @enterprise only and required license to test.
srkgupta
added
4: Reviews Complete
amyblais
added
Changelog/Not Needed
* Cypress/E2E: Fix disabled bot accounts section verification * Remove extra cy.apiAdminLogin * Moved cy.apiAdminLogin to top of beforeEach * Moved bot cannot login case to enterprise * Moved function to bottom
Summary
Ticket Link
NA