Video 3727 cypress testing

[olipyskoty]

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

• I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

• VIDEO-3727

Description

This PR adds cypress tests for the chat feature. The tests test the ability to send and receive messages (implicitly) as well as the chat window's scrolling behavior. We also found a small bug in the<MessageListScrollingContainer /> when checking whether to scroll the component to the bottom in the componentDidUpdate() method. We made it so the component only auto-scrolls when the user has a new message and they are already scrolled to the bottom before receiving that message.

Burndown

Before review

• Updated CHANGELOG.md if necessary
• Added unit tests if necessary
• Updated affected documentation
• Verified locally with npm test
• Manually sanity tested running locally
• Included screenshot as PR comment (if needed)
• Ready for review

Before merge

• Got one or more +1s
• Re-tested if necessary

[timmydoza]

Nice work! Left some comments.

Also, my cross-browser fixes are now on the feature/conversations branch. Can you merge that branch in to this one? It has updates to the MessageListScrollContainer component, and I'd like to make sure that these tests still pass with those updates.

[timmydoza]

When it comes to testing, one general rule is that the order of the tests should not matter. The tests should be written in a way where their order can change and they will still pass. This just helps the tests to be less brittle.

Looks like all the tests rely on the welcome to the chat! message being present, so I think it would be best to move that command up to the before() block. This ensures that the message will be present before all tests are run:

Suggested change

	before(() => {
	cy.get('[data-cy-chat-button]').click();
	});
	after(() => {
	cy.get('[data-cy-chat-button]').click();
	});
	it('should see "1 new message" button when not scrolled to bottom of chat and a new message is received', () => {
	cy.task('sendAMessage', {
	name: 'test1',
	message: 'welcome \n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n to the chat!',
	});
	cy.contains('welcome');
	cy.get('[data-cy-message-list-inner-scroll]').scrollTo(0, 0);
	cy.task('sendAMessage', { name: 'test1', message: 'how is it going?' });
	cy.get('[data-cy-message-list-outer]').should('contain', '1 new message');
	});
	before(() => {
	cy.get('[data-cy-chat-button]').click();
	cy.task('sendAMessage', {
	name: 'test1',
	message: 'welcome \n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n to the chat!',
	});
	cy.contains('welcome');
	});
	after(() => {
	cy.get('[data-cy-chat-button]').click();
	});
	it('should see "1 new message" button when not scrolled to bottom of chat and a new message is received', () => {
	cy.get('[data-cy-message-list-inner-scroll]').scrollTo(0, 0);
	cy.task('sendAMessage', { name: 'test1', message: 'how is it going?' });
	cy.get('[data-cy-message-list-outer]').should('contain', '1 new message');
	});

[timmydoza]

Same here. I think this test should send a new message to make the "1 new message" button appear. This way it doesn't matter if the previous test already did this or not:

Suggested change

	it('should scroll to bottom of chat when "1 new message button" is clicked on', () => {
	cy.get('[data-cy-message-list-inner-scroll]').scrollTo(0, 0);
	cy.get('[data-cy-new-message-button]')
	.should('be.visible')
	.click();
	cy.get('[data-cy-message-list-outer]')
	.contains('how is it going')
	.should('be.visible');
	cy.get('[data-cy-message-list-inner-scroll]').should($el => {
	expect($el.prop('scrollHeight')).to.equal($el.prop('scrollTop') + $el.prop('clientHeight'));
	});
	});
	it('should scroll to bottom of chat when "1 new message button" is clicked on', () => {
	cy.get('[data-cy-message-list-inner-scroll]').scrollTo(0, 0);
	cy.task('sendAMessage', { name: 'test1', message: 'Ahoy!' });
	cy.contains('Ahoy!');
	cy.get('[data-cy-new-message-button]')
	.should('be.visible')
	.click();
	cy.get('[data-cy-message-list-outer]')
	.contains('Ahoy!')
	.should('be.visible');
	cy.get('[data-cy-message-list-inner-scroll]').should($el => {
	expect($el.prop('scrollHeight')).to.equal($el.prop('scrollTop') + $el.prop('clientHeight'));
	});
	});

[timmydoza]

Nit: I think that cy.get('[data-cy-message-list-outer]').contains('foo') is the same as cy.get('[data-cy-message-list-inner-scroll]').contains('foo') since both of those divs wrap the same messages. So let's change all instances of [data-cy-message-list-outer] to [data-cy-message-list-inner-scroll] and remove the [data-cy-message-list-outer] attribute altogether.

[timmydoza]

Nice comment! 🎉