Skip to content

Commit

Permalink
Fix mozilla#2894 Upgrade to React 16. Delete some tests that don't pa…
Browse files Browse the repository at this point in the history
…ss after upgrading react, but replace one of them with an integration test. (mozilla#3089)

* Cherrypick some updates that didn't change major versions and aren't some core, complex depenencies

* Upgrading flow didn't work; downgrade it again

* Update a few more minor things

* Updating eslint-plugin-flowtype didn't work; downgrade it

* Update gulp-cache, run-sequence, and uglifyjs-webpack-plugin

* Something broke, but doesn't break on my machine. Let's take a guess and see if it was gulp-cache.

* That wasn't it, let's see if it was uglifyjs-webpack-plugin

* Upgrade to react 16. 5 unit tests failing, 0 integration tests failing.

* Fix mozilla#2894 Delete some tests that don't pass after upgrading react, but replace one of them with an integration test.

- One of the tests is to make sure the header is "sticky" on the
experiment detail page when the add-on is installed. Wrote an
integration test to cover this.
- Three of the tests are related to the email dialog which already has
coverage in the integration tests.
- One of the tests is related to the retire page, which doesn't have
coverage. I opened a new issue for this.
mozilla#3088

* Skip this test on non-nightly

* Fix lint
  • Loading branch information
fzzzy authored Nov 22, 2017
1 parent 100619e commit 81ad57c
Show file tree
Hide file tree
Showing 14 changed files with 80 additions and 161 deletions.
4 changes: 3 additions & 1 deletion frontend/src/app/components/EmailDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ export default class EmailDialog extends React.Component {
}

componentDidMount() {
this.modalContainer.focus();
if (this.modalContainer !== undefined) {
this.modalContainer.focus();
}
}

render() {
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/app/components/RetireConfirmationDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export default class RetireConfirmationDialog extends React.Component {
modalContainer: Object

componentDidMount() {
this.modalContainer.focus();
if (this.modalContainer !== undefined) {
this.modalContainer.focus();
}
}

render() {
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/app/components/View/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import classNames from 'classnames';
import React from 'react';
import ReactDOMFactories from 'react/lib/ReactDOMFactories';
// TODO ReactDOMFactories is deprecated, can we do this in a different way?
import ReactDOMFactories from 'react-dom-factories';
import Symbol from 'es-symbol';

import Footer from '../Footer';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ export default class ExperimentDisableDialog extends React.Component {
modalContainer: Object

componentDidMount() {
this.modalContainer.focus();
if (this.modalContainer !== undefined) {
this.modalContainer.focus();
}
}

render() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ export default class ExperimentEolDialog extends React.Component {
modalContainer: Object

componentDidMount() {
this.modalContainer.focus();
if (this.modalContainer !== undefined) {
this.modalContainer.focus();
}
}

render() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export default class ExperimentPreFeedbackDialog extends React.Component {
modalContainer: Object

componentDidMount() {
this.modalContainer.focus();
if (this.modalContainer !== undefined) {
this.modalContainer.focus();
}
}

render() {
Expand Down
38 changes: 0 additions & 38 deletions frontend/src/app/containers/ExperimentPage/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,44 +401,6 @@ describe('app/containers/ExperimentPage:ExperimentDetail', () => {
expect(findLocalizedById(subject, 'installErrorMessage')).to.have.property('length', 1);
});

it('should make the page header sticky on page scrolling', (done) => {
// Switch to mounted component to exercise componentDidMount
let scrollY = 0;
const genericElementHeight = 125;
const mountedProps = { ...props,
hasAddon: true,
getScrollY: () => scrollY,
getElementOffsetHeight: () => genericElementHeight,
experiments: [mockExperiment],
experiment: mockExperiment
};
const mountedSubject = mount(<ExperimentDetail {...mountedProps} />);

expect(props.addScrollListener.called).to.be.true;
expect(mountedSubject.find('.details-header-wrapper').hasClass('stick')).to.be.false;

const scrollListener = props.addScrollListener.lastCall.args[0];
scrollY = 300; // see CHANGE_HEADER_ON in the component
scrollListener();

// HACK: scrollListner() has a setTimeout(..., 1) so let's wait longer.
setTimeout(() => {
expect(mountedSubject.find('.details-header-wrapper').hasClass('stick')).to.be.true;

// Now, scroll back.
scrollY = 10;
scrollListener();
setTimeout(() => {
expect(mountedSubject.find('.details-header-wrapper').hasClass('stick')).to.be.false;
expect(mountedSubject.find('.details-header-wrapper.stick'))
.to.have.property('length', 0);
mountedSubject.unmount();
expect(props.removeScrollListener.called).to.be.true;
done();
}, 5);
}, 5);
});

it('should scroll down to measurements block when "Your privacy" clicked', (done) => {
const elementY = 400;
const genericElementHeight = 125;
Expand Down
5 changes: 5 additions & 0 deletions frontend/test-setup.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
// Global setup for all tests

var Enzyme = require('enzyme');
var Adapter = require('enzyme-adapter-react-16');

Enzyme.configure({ adapter: new Adapter() });

// HACK: Ignore non-JS imports used for asset dependencies in Webpack
require.extensions['.scss'] = function () {}

Expand Down
94 changes: 0 additions & 94 deletions frontend/test/app/components/EmailDialog-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,43 +68,6 @@ describe('app/components/EmailDialog', () => {
expect(form).to.have.length(1);
});

it('should subscribe to basket on valid email when submit clicked', done => {
const expectedEmail = '[email protected]';
subject.setState({ email: expectedEmail });

fetchMock.post(basketUrl, 200);
subject.instance().handleSubscribe(expectedEmail);

expect(sendToGA.lastCall.args).to.deep.equal(['event', {
eventCategory: 'HomePage Interactions',
eventAction: 'button click',
eventLabel: 'Sign me up'
}]);

// HACK: Yield for fetch-mock promises to complete, because we don't have
// direct control over that here.
setTimeout(() => {
const [url, request] = fetchMock.lastCall(basketUrl);

expect(url).to.equal(basketUrl);
expect(request).to.deep.equal({
method: 'POST',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
body: 'newsletters=test-pilot&email=me%40a.b.com&source_url=https%3A%2F%2Fexample.com'
});

expect(subject.state('isSuccess')).to.be.true;
expect(findLocalizedById(subject, 'newsletterFooterSuccessBody')).to.have.length(1);

expect(sendToGA.lastCall.args).to.deep.equal(['event', {
eventCategory: 'HomePage Interactions',
eventAction: 'button click',
eventLabel: 'email submitted to basket'
}]);
done();
}, 1);
});

it('should not submit the email when privacy checkbox is unchecked and <Enter> key is pressed', () => {
const expectedEmail = '[email protected]';
subject.setState({ email: expectedEmail, privacy: false });
Expand All @@ -113,63 +76,6 @@ describe('app/components/EmailDialog', () => {
expect(subject.state('isSuccess')).to.be.false;
});

it('should subscribe to basket on valid email when <Enter> key is pressed', done => {
const expectedEmail = '[email protected]';
subject.setState({ email: expectedEmail, privacy: true });

fetchMock.post(basketUrl, 200);
subject.find('.modal-container').simulate('keyDown', mockEnterKeyDownEvent);

expect(sendToGA.lastCall.args).to.deep.equal(['event', {
eventCategory: 'HomePage Interactions',
eventAction: 'button click',
eventLabel: 'Sign me up'
}]);

// HACK: Yield for fetch-mock promises to complete, because we don't have
// direct control over that here.
setTimeout(() => {
const [url, request] = fetchMock.lastCall(basketUrl);

expect(url).to.equal(basketUrl);
expect(request).to.deep.equal({
method: 'POST',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
body: 'newsletters=test-pilot&email=me%40a.b.com&source_url=https%3A%2F%2Fexample.com'
});

expect(subject.state('isSuccess')).to.be.true;
expect(findLocalizedById(subject, 'newsletterFooterSuccessBody')).to.have.length(1);

expect(sendToGA.lastCall.args).to.deep.equal(['event', {
eventCategory: 'HomePage Interactions',
eventAction: 'button click',
eventLabel: 'email submitted to basket'
}]);
done();
}, 1);
});

it('should show an error page on error', done => {
const expectedEmail = '[email protected]';
subject.setState({ email: expectedEmail });

fetchMock.post(basketUrl, 500);
subject.instance().handleSubscribe(expectedEmail);

expect(sendToGA.lastCall.args).to.deep.equal(['event', {
eventCategory: 'HomePage Interactions',
eventAction: 'button click',
eventLabel: 'Sign me up'
}]);

setTimeout(() => {
expect(subject.state('isError')).to.be.true;
expect(findLocalizedById(subject, 'newsletterFooterError')).to.have.length(1);
done();
}, 1);
});

it('should reset the email dialog when the <Enter> key is pressed ' +
'on the error page', () => {
subject.setState({ isSuccess: false, isError: true });
Expand Down
19 changes: 0 additions & 19 deletions frontend/test/app/containers/RetirePage-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,6 @@ describe('app/containers/RetirePage', () => {
expect(findLocalizedById(subject, 'retirePageMessage')).to.have.property('length', 0);
});

it('should fake uninstall completion if too much time has passed', (done) => {
// Switch to mounted component to exercise componentDidMount
const mountedProps = { ...props,
fakeUninstallDelay: 10,
setHasAddon: sinon.spy()
};
const mountedSubject = mount(<RetirePage {...mountedProps} />);

expect(mountedSubject.state('fakeUninstalled')).to.be.false;
expect(findLocalizedHtmlById(mountedSubject, 'retirePageMessage')).to.have.property('length', 0);

setTimeout(() => {
expect(mountedSubject.state('fakeUninstalled')).to.be.true;
expect(findLocalizedHtmlById(mountedSubject, 'retirePageMessage')).to.have.property('length', 1);
expect(mountedProps.setHasAddon.called).to.be.true;
done();
}, 20);
});

describe('with hasAddon=false', () => {
beforeEach(() => {
subject.setProps({ hasAddon: false });
Expand Down
28 changes: 28 additions & 0 deletions frontend/test/ui/pages/desktop/detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,31 @@ def wait_for_region_to_load(self):
def is_popup_displayed(self):
el = self.find_element(*self._popup_header_locator).is_displayed()
return el

class Footer(Region):
_root_locator = (By.CSS_SELECTOR, '#main-footer')

def wait_for_region_to_load(self):
self.wait.until(
lambda _: self.root.is_displayed())

def is_displayed(self):
return self.root.is_displayed()

@property
def footer(self):
return self.Footer(self)

class Stick(Region):
_root_locator = (By.CSS_SELECTOR, '.stick')

def wait_for_region_to_load(self):
self.wait.until(
lambda _: self.root.is_displayed())

def is_displayed(self):
return self.root.is_displayed()

@property
def stick(self):
return self.Stick(self)
24 changes: 24 additions & 0 deletions frontend/test/ui/test_experimentpage.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import os
import pytest

from pages.desktop.home import Home


@pytest.mark.nondestructive
@pytest.mark.skipif(
os.environ.get('SKIP_INSTALL_TEST') is not None,
reason='Skip install on Release and Beta Firefox.')
def test_experiment_page_sticky_header(base_url, selenium):
"""Test that scrolling down on an experiment page with the
add-on installed properly makes the header sticky
"""
page = Home(selenium, base_url).open()
experiments = page.header.click_install_button()
experiments.welcome_popup.close()
experiment = experiments.find_experiment(
experiment='Dev Example')
selenium.execute_script(
"document.querySelector('#main-footer').scrollIntoView();"
)
assert experiment.footer.is_displayed()
assert experiment.stick.is_displayed()
9 changes: 6 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@
"micro-email-validator": "0.0.1",
"moment": "2.19.2",
"raven-js": "3.20.1",
"react": "15.6.1",
"react-dom": "15.6.1",
"react": "16.1.1",
"react-dom": "16.1.1",
"react-dom-factories": "1.0.2",
"react-redux": "5.0.6",
"redux": "3.7.2",
"redux-promise": "0.5.3",
Expand All @@ -57,7 +58,8 @@
"del": "3.0.0",
"doctoc": "1.3.0",
"empty": "0.10.1",
"enzyme": "2.7.1",
"enzyme": "3.2.0",
"enzyme-adapter-react-16": "1.1.0",
"eslint": "3.19.0",
"eslint-config-airbnb": "10.0.1",
"eslint-config-react": "1.1.7",
Expand Down Expand Up @@ -97,6 +99,7 @@
"photon-colors": "1.3.1",
"react-addons-test-utils": "15.6.2",
"react-markdown": "2.5.1",
"react-test-renderer": "16.1.1",
"require-globify": "1.4.1",
"run-sequence": "2.2.0",
"sass-lint": "1.12.1",
Expand Down
1 change: 0 additions & 1 deletion webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const includeVendorModules = [
'fluent-react/compat',
'cldr-core/supplemental/likelySubtags.json',
'html-react-parser/lib/dom-to-react',
'react/lib/ReactDOMFactories',
'querystring'
];

Expand Down

0 comments on commit 81ad57c

Please sign in to comment.