From e79ad764bd41cf989252b1dda32c4934f41ef8a5 Mon Sep 17 00:00:00 2001 From: Jacob Herrington Date: Fri, 22 Nov 2019 13:23:57 -0600 Subject: [PATCH] Onboarding test love (#4878) * Reduce code duplication in onboarding tests * Update variable names for consistency * Improve test descriptions and format * Refactor a useless test This test was fairly pointless, but hopefully with this refactor it could detect a regression? Although, I think it's really just testing the that the Preact framework's state mechanism is working the way we expect it to, but it might be worth leaving this test in. --- .../onboarding/__tests__/Onboarding.test.jsx | 211 ++++++++++-------- 1 file changed, 122 insertions(+), 89 deletions(-) diff --git a/app/javascript/onboarding/__tests__/Onboarding.test.jsx b/app/javascript/onboarding/__tests__/Onboarding.test.jsx index 28a170c85..cdebb524e 100644 --- a/app/javascript/onboarding/__tests__/Onboarding.test.jsx +++ b/app/javascript/onboarding/__tests__/Onboarding.test.jsx @@ -16,6 +16,19 @@ function flushPromises() { return new Promise(resolve => setImmediate(resolve)); } +function initializeSlides(currentSlide, dataUser = null, mockData = null) { + const onboardingSlides = deep(); + + if (mockData) { + fetch.once(mockData); + } + + document.body.setAttribute('data-user', dataUser); + onboardingSlides.setState({ currentSlide }); + + return onboardingSlides; +} + describe('', () => { beforeAll(() => { expect.extend(toHaveNoViolations); @@ -24,7 +37,7 @@ describe('', () => { fetch.resetMocks(); }); - const fakeTagResponse = JSON.stringify([ + const fakeTagsResponse = JSON.stringify([ { bg_color_hex: '#000000', id: 715, @@ -44,8 +57,7 @@ describe('', () => { text_color_hex: '#ffffff', }, ]); - - const fakeUsersToFollowResponse = JSON.stringify([ + const fakeUsersResponse = JSON.stringify([ { id: 1, name: 'Ben Halpern', @@ -62,16 +74,15 @@ describe('', () => { profile_image_url: 'dev.jpg', }, ]); - const dataUser = JSON.stringify({ followed_tag_names: ['javascript'], }); describe('IntroSlide', () => { let onboardingSlides; + beforeEach(() => { - document.body.setAttribute('data-user', null); - onboardingSlides = deep(); + onboardingSlides = initializeSlides(0); }); test('renders properly', () => { @@ -84,7 +95,7 @@ describe('', () => { expect(results).toHaveNoViolations(); }); - test('should move to the next slide upon clicking the next button', () => { + test('should advance', () => { onboardingSlides.find('.next-button').simulate('click'); expect(onboardingSlides.state().currentSlide).toBe(1); }); @@ -92,61 +103,81 @@ describe('', () => { describe('EmailTermsConditionsForm', () => { let onboardingSlides; + const codeOfConductCheckEvent = { + target: { + value: 'checked_code_of_conduct', + name: 'checked_code_of_conduct', + }, + }; + const termsAndConditionsCheckEvent = { + target: { + value: 'checked_terms_and_conditions', + name: 'checked_terms_and_conditions', + }, + }; + const updateCodeOfConduct = () => { + onboardingSlides + .find('#checked_code_of_conduct') + .simulate('change', codeOfConductCheckEvent); + }; + const updateTermsAndConditions = () => { + onboardingSlides + .find('#checked_terms_and_conditions') + .simulate('change', termsAndConditionsCheckEvent); + }; + beforeEach(() => { - document.body.setAttribute('data-user', dataUser); - onboardingSlides = deep(); - onboardingSlides.setState({ currentSlide: 1 }); + onboardingSlides = initializeSlides(1, dataUser); }); test('renders properly', () => { expect(onboardingSlides).toMatchSnapshot(); }); - test('should not move if code of conduct is not agreed to', () => { - onboardingSlides.find('.next-button').simulate('click'); - expect(onboardingSlides.state().currentSlide).toBe(1); - }); - - test('should not move if terms and conditions are not met', () => { - const event = { - target: { - value: 'checked_code_of_conduct', - name: 'checked_code_of_conduct', - }, - }; - onboardingSlides - .find('#checked_code_of_conduct') - .simulate('change', event); - onboardingSlides.find('.next-button').simulate('click'); - expect(onboardingSlides.state().currentSlide).toBe(1); - }); - - test('should move to the next slide if code of conduct and terms and conditions are met', async () => { - fetch.once({}); - onboardingSlides.find('#checked_code_of_conduct').simulate('change', { - target: { - value: 'checked_code_of_conduct', - name: 'checked_code_of_conduct', - }, - }); - - onboardingSlides - .find('#checked_terms_and_conditions') - .simulate('change', { - target: { - value: 'checked_terms_and_conditions', - name: 'checked_terms_and_conditions', - }, - }); - + // Arguably this test is actually just testing the Preact framework + // but for the sake of detecting a regression I am refactoring it instead + // of removing it (@jacobherrington) + test('should track state changes', () => { const emailTerms = onboardingSlides.find(); + + expect(emailTerms.state('checked_code_of_conduct')).toBe(false); + expect(emailTerms.state('checked_terms_and_conditions')).toBe(false); + + updateCodeOfConduct(); + updateTermsAndConditions(); + expect(emailTerms.state('checked_code_of_conduct')).toBe(true); + expect(emailTerms.state('checked_terms_and_conditions')).toBe(true); + }); + + test('should not advance if required boxes are not checked', () => { + // When none of the boxes are checked + onboardingSlides.find('.next-button').simulate('click'); + expect(onboardingSlides.state().currentSlide).toBe(1); + + // When only the code of conduct is checked + updateCodeOfConduct(); + expect(onboardingSlides.state().currentSlide).toBe(1); + + // When only the terms and conditions are checked + updateCodeOfConduct(); + updateTermsAndConditions(); + onboardingSlides.find('.next-button').simulate('click'); + expect(onboardingSlides.state().currentSlide).toBe(1); + }); + + test('should advance if required boxes are checked', async () => { + fetch.once({}); + + updateCodeOfConduct(); + updateTermsAndConditions(); + onboardingSlides.find('.next-button').simulate('click'); await flushPromises(); expect(onboardingSlides.state().currentSlide).toBe(2); }); - it('should move to the previous slide upon clicking the back button', () => { + it('should step backward', () => { onboardingSlides.find('.back-button').simulate('click'); expect(onboardingSlides.state().currentSlide).toBe(0); }); @@ -155,46 +186,45 @@ describe('', () => { describe('BioForm', () => { let onboardingSlides; const meta = document.createElement('meta'); + meta.setAttribute('name', 'csrf-token'); document.body.appendChild(meta); beforeEach(() => { - onboardingSlides = deep(); - onboardingSlides.setState({ currentSlide: 2 }); - document.body.setAttribute('data-user', dataUser); + onboardingSlides = initializeSlides(2, dataUser); }); test('renders properly', () => { expect(onboardingSlides).toMatchSnapshot(); }); - it('should move to the previous slide upon clicking the back button', () => { - onboardingSlides.find('.back-button').simulate('click'); - expect(onboardingSlides.state().currentSlide).toBe(1); - }); - - test('forms can be filled and submitted', async () => { + test('should allow user to fill forms and advance', async () => { fetch.once({}); const bioForm = onboardingSlides.find(); const event = { target: { value: 'my bio', name: 'summary' } }; + onboardingSlides.find('textarea').simulate('change', event); - expect(bioForm.state('summary')).toBe('my bio'); + expect(bioForm.state('summary')).toBe(event.target.value); bioForm.find('.next-button').simulate('click'); await flushPromises(); expect(onboardingSlides.state().currentSlide).toBe(3); }); + + it('should step backward', () => { + onboardingSlides.find('.back-button').simulate('click'); + expect(onboardingSlides.state().currentSlide).toBe(1); + }); }); describe('PersonalInformationForm', () => { let onboardingSlides; const meta = document.createElement('meta'); + meta.setAttribute('name', 'csrf-token'); document.body.appendChild(meta); beforeEach(() => { - onboardingSlides = deep(); - onboardingSlides.setState({ currentSlide: 3 }); - document.body.setAttribute('data-user', dataUser); + onboardingSlides = initializeSlides(3, dataUser); }); test('renders properly', () => { @@ -206,30 +236,36 @@ describe('', () => { expect(onboardingSlides.state().currentSlide).toBe(2); }); - test('forms can be filled and submitted', async () => { + test('should allow user to fill forms and advance', async () => { fetch.once({}); + const personalInfoForm = onboardingSlides.find(); const locationEvent = { target: { value: 'my location', name: 'location' }, }; - onboardingSlides.find('#location').simulate('change', locationEvent); - const titleEvent = { target: { value: 'my title', name: 'employment_title' }, }; - onboardingSlides.find('#employment_title').simulate('change', titleEvent); - const employerEvent = { target: { value: 'my employer name', name: 'employer_name' }, }; + + onboardingSlides.find('#location').simulate('change', locationEvent); + onboardingSlides.find('#employment_title').simulate('change', titleEvent); onboardingSlides.find('#employer_name').simulate('change', employerEvent); - expect(personalInfoForm.state('location')).toBe('my location'); - expect(personalInfoForm.state('employment_title')).toBe('my title'); - expect(personalInfoForm.state('employer_name')).toBe('my employer name'); + expect(personalInfoForm.state('location')).toBe( + locationEvent.target.value, + ); + expect(personalInfoForm.state('employment_title')).toBe( + titleEvent.target.value, + ); + expect(personalInfoForm.state('employer_name')).toBe( + employerEvent.target.value, + ); personalInfoForm.find('.next-button').simulate('click'); - fetch.once(fakeTagResponse); + fetch.once(fakeTagsResponse); await flushPromises(); expect(onboardingSlides.state().currentSlide).toBe(4); }); @@ -237,11 +273,9 @@ describe('', () => { describe('FollowTags', () => { let onboardingSlides; + beforeEach(async () => { - document.body.setAttribute('data-user', dataUser); - onboardingSlides = deep(); - fetch.once(fakeTagResponse); - onboardingSlides.setState({ currentSlide: 4 }); + onboardingSlides = initializeSlides(4, dataUser, fakeTagsResponse); await flushPromises(); }); @@ -249,25 +283,26 @@ describe('', () => { expect(onboardingSlides).toMatchSnapshot(); }); - test('it should render three tags', async () => { + test('should render three tags', async () => { expect(onboardingSlides.find('.tag').length).toBe(3); }); - test('adding a tag and submitting works', async () => { + test('should allow a user to add a tag', async () => { fetch.once({}); const followTags = onboardingSlides.find(); + onboardingSlides .find('.tag') .first() .simulate('click'); expect(followTags.state('selectedTags').length).toBe(1); onboardingSlides.find('.next-button').simulate('click'); - fetch.once(fakeUsersToFollowResponse); + fetch.once(fakeUsersResponse); await flushPromises(); expect(onboardingSlides.state().currentSlide).toBe(5); }); - it('should move to the previous slide upon clicking the back button', () => { + it('should step backward', () => { onboardingSlides.find('.back-button').simulate('click'); expect(onboardingSlides.state().currentSlide).toBe(3); }); @@ -275,11 +310,9 @@ describe('', () => { describe('FollowUsers', () => { let onboardingSlides; + beforeEach(async () => { - document.body.setAttribute('data-user', dataUser); - onboardingSlides = deep(); - fetch.once(fakeUsersToFollowResponse); - onboardingSlides.setState({ currentSlide: 5 }); + onboardingSlides = initializeSlides(5, dataUser, fakeUsersResponse); await flushPromises(); }); @@ -287,13 +320,14 @@ describe('', () => { expect(onboardingSlides).toMatchSnapshot(); }); - test('it should render three users', async () => { + test('should render three users', async () => { expect(onboardingSlides.find('.user').length).toBe(3); }); - test('adding a user and submitting works', async () => { + test('should allow a user to select and advance', async () => { fetch.once({}); const followUsers = onboardingSlides.find(); + onboardingSlides .find('.user') .first() @@ -304,8 +338,8 @@ describe('', () => { expect(onboardingSlides.state().currentSlide).toBe(6); }); - it('should move to the previous slide upon clicking the back button', async () => { - fetch.once(fakeTagResponse); + it('should step backward', async () => { + fetch.once(fakeTagsResponse); onboardingSlides.find('.back-button').simulate('click'); await flushPromises(); expect(onboardingSlides.state().currentSlide).toBe(4); @@ -314,10 +348,9 @@ describe('', () => { describe('CloseSlide', () => { let onboardingSlides; + beforeEach(() => { - document.body.setAttribute('data-user', null); - onboardingSlides = deep(); - onboardingSlides.setState({ currentSlide: 6 }); + onboardingSlides = initializeSlides(6); }); test('renders properly', () => {