Eliminate yarn test spam #1024

lieut-data · 2018-03-28T21:42:26Z

Summary

Finding it hard to iterate on webapp unit tests when there's so many console logs and warnings scrolling by. This set of commits eliminates them -- comments below where it's not obvious how or why.

It also adds a global override on console.(log|warn|error) to throw an Error and prevent future such issues.

Ticket Link

None.

Checklist

Ran make check-style to check for style errors (required for all pull requests)
Ran make test to ensure unit and component tests passed
Added or updated unit tests (required for all new features)

This was originally added in a bid to track down a now resolved issue, and thus is no longer needed.

See localForage/localForage#722.

…trap component

lieut-data · 2018-03-28T21:43:11Z

components/profile_popover/profile_popover.jsx

@@ -200,6 +200,7 @@ class ProfilePopover extends React.Component {
 delete popoverProps.hasMention;
 delete popoverProps.dispatch;
 delete popoverProps.enableWebrtc;
+ delete popoverProps.showEmailAddress;


Injected by the connector but wasn't removed, like enableWebrtc, and ended up incorrectly as a DOM property.

I think we are not using showEmailAddress at all, so, the solution would be remove that from the connect function.

Ooh, I like that better -- somehow, I thought this was being passed in via a plugin, but you're right that it's only part of the pluggable unit tests.

lieut-data · 2018-03-28T21:43:27Z

components/sidebar/sidebar.jsx

@@ -76,12 +76,12 @@ export default class Sidebar extends React.PureComponent {
 /**
 * Current team object
 */
- currentTeam: PropTypes.object.isRequired,
+ currentTeam: PropTypes.object,


The code handles these not being present.

lieut-data · 2018-03-28T21:43:39Z

package.json

@@ -88,6 +88,7 @@
 "image-webpack-loader": "3.6.0",
 "imports-loader": "0.7.1",
 "jest": "22.1.4",
+ "jest-canvas-mock": "1.0.2",


So that the doughtnut chart doesn't throw errors about getContext not being defined.

lieut-data · 2018-03-28T21:44:46Z

store/index.js

@@ -73,7 +73,7 @@ export default function configureStore(initialState) {
 const storage = localforage;
 const KEY_PREFIX = 'reduxPersist:';

- localforage.ready(() => {
+ localforage.ready().then(() => {


See localForage/localForage#722 for more details.

lieut-data · 2018-03-28T21:45:33Z

store/index.js

@@ -135,6 +135,11 @@ export default function configureStore(initialState) {
 });
 }
 });
+ }).catch((error) => {
+ store.dispatch({
+ type: General.STORE_REHYDRATION_FAILED,


I've got a pending PR to add this constant to mattermost-redux... if we think we need it. No one is going to handle it right now. Feedback requested!

I'm not sure if I prefer simply a log message for now. Anyway, I think it must be in the webapp redux flow, not in the mattermost-redux one. Because all the storage thing is handle independently from RN app and webapp.

I'll move the constant locally -- the log message would defeat the point of this change, at present ;P

hahaha, you are completely right, I'm an idiot :)

lieut-data · 2018-03-28T21:45:47Z

tests/components/channel_notifications_modal.test.jsx

@@ -10,8 +10,18 @@ describe('components/channel_notifications_modal/ChannelNotificationsModal', ()
 show: true,
 onHide: () => {}, //eslint-disable-line no-empty-function
 channel: {id: 'channel_id', display_name: 'channel_display_name'},
- channelMember: {notify_props: {desktop: NotificationLevels.ALL}},
- currentUser: {id: 'current_user_id', notify_props: {desktop: NotificationLevels.ALL}},
+ channelMember: {


Required props.

lieut-data · 2018-03-28T21:46:02Z

tests/setup.js

@@ -20,3 +20,15 @@ Object.defineProperty(document, 'queryCommandSupported', {
 Object.defineProperty(document, 'execCommand', {
 value: (cmd) => supportedCommands.includes(cmd),
 });
+
+beforeEach(() => {


Don't let this regress in the future.

lieut-data · 2018-03-28T21:46:17Z

utils/network.js

@@ -8,7 +8,6 @@ const MIN_NETWORK_RETRY_TIME = 3000; // 3 sec
 const MAX_NETWORK_RETRY_TIME = 300000; // 5 mins

 function handle(callback, online) {
- console.log('Network status set to ' + online); //eslint-disable-line no-console


Checked with author: no longer needed.

lieut-data · 2018-03-28T21:46:20Z

utils/storage_utils.js

@@ -9,8 +9,6 @@ export function getPrefix(state) {
 }
 }

- console.warn('Storage tried to operate without user present'); //eslint-disable-line no-console


Checked with author: no longer needed.

jespino

I think we must remove the showEmailAddress property. The other comment related to REHYDRATATION, I'm ok with your proposed solution (in mattermost-webapp, not in mattermost-redux).

enahum · 2018-04-03T10:17:19Z

Agree with @jespino

…er bootstrap component" This reverts commit 73b5fcb.

lieut-data · 2018-04-03T11:51:05Z

Done, thanks @jespino and @enahum :)

jespino

👍

lieut-data · 2018-04-03T13:20:02Z

Hmm, I'm confused -- not seeing these test failures locally, nor in the previous build. I'll investigate.

…-spam

lieut-data · 2018-04-03T13:47:19Z

Looks like just some recent regressions on exactly this front, now also fixed. Going forward, this will be caught by individual developers.

#1024) * MM-21290: Checks for license AND config prior to overriding user's display name account setting. * MM-21290: Adds license to mock.

lieut-data added 8 commits March 28, 2018 17:32

remove "Network status set to (true|false)" message

3604978

This was originally added in a bid to track down a now resolved issue, and thus is no longer needed.

work around UnhandledPromiseRejectionWarning errors from localforage

d548f3d

See localForage/localForage#722.

fix missing props in tests

c99562f

skip passing showEmailAddress from ProfilePopver to the Popover boots…

73b5fcb

…trap component

eliminate unneeded warning in getPrefix

b331b43

fix spurious state in rhs reducer tests

9a49fde

leverage jest-canvas-mock to avoid warnings from the doughnut chart

dc6d2a4

fail the build on console messages

b7c99d9

lieut-data requested review from jespino and enahum March 28, 2018 21:46

lieut-data commented Mar 28, 2018

View reviewed changes

jwilander added the 2: Dev Review Requires review by a core commiter label Mar 28, 2018

jespino suggested changes Apr 3, 2018

View reviewed changes

lieut-data added 3 commits April 3, 2018 07:43

Revert "skip passing showEmailAddress from ProfilePopver to the Popov…

ab47e5b

…er bootstrap component" This reverts commit 73b5fcb.

just remove showEmailAddress in the profile popover unit tests

688c6b8

define STORE_REHYDRATION_FAILED locally vs. in mattermost-redux, for now

7642e80

jespino approved these changes Apr 3, 2018

View reviewed changes

enahum approved these changes Apr 3, 2018

View reviewed changes

Merge remote-tracking branch 'origin/master' into eliminate-yarn-test…

e480ec1

…-spam

fix recently broken unit tests

b209749

lieut-data force-pushed the eliminate-yarn-test-spam branch from cb41cfe to b209749 Compare April 3, 2018 13:50

lieut-data merged commit 61e2433 into master Apr 3, 2018

lieut-data deleted the eliminate-yarn-test-spam branch April 3, 2018 14:02

amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Apr 3, 2018

lindalumitchell added the Tests/Not Needed Does not require new release tests label Apr 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eliminate yarn test spam #1024

Eliminate yarn test spam #1024

lieut-data commented Mar 28, 2018

lieut-data Mar 28, 2018

jespino Apr 3, 2018

lieut-data Apr 3, 2018

lieut-data Mar 28, 2018

lieut-data Mar 28, 2018

lieut-data Mar 28, 2018

lieut-data Mar 28, 2018

jespino Apr 3, 2018

lieut-data Apr 3, 2018

jespino Apr 3, 2018

lieut-data Mar 28, 2018

lieut-data Mar 28, 2018

lieut-data Mar 28, 2018

lieut-data Mar 28, 2018

jespino left a comment

enahum commented Apr 3, 2018

lieut-data commented Apr 3, 2018

jespino left a comment

lieut-data commented Apr 3, 2018

lieut-data commented Apr 3, 2018

Eliminate yarn test spam #1024

Eliminate yarn test spam #1024

Conversation

lieut-data commented Mar 28, 2018

Summary

Ticket Link

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jespino left a comment

Choose a reason for hiding this comment

enahum commented Apr 3, 2018

lieut-data commented Apr 3, 2018

jespino left a comment

Choose a reason for hiding this comment

lieut-data commented Apr 3, 2018

lieut-data commented Apr 3, 2018