New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fix flow, jest unit test errors #1068

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@kunall17
Contributor

kunall17 commented Aug 20, 2017

Currently disabled the flow type for the Avatar.js file

Now getting error's in selector test's

yarn run jest subscriptionSelectors
yarn run v0.27.5
warning ../../../package.json: No license field
$ "/Users/kunall17/Project/react/zulip-mobile/node_modules/.bin/jest" "subscriptionSelectors"
 FAIL  src/subscriptions/__tests__/subscriptionSelectors-test.js
   Test suite failed to run

    Selector creators expect all input-selectors to be functions, instead received the following types: [undefined, function, function]
      
      at getDependencies (node_modules/reselect/lib/index.js:53:11)
      at node_modules/reselect/lib/index.js:71:24
      at Object.<anonymous> (src/unread/unreadSelectors.js:77:93)
      at Object.<anonymous> (src/conversations/conversationsSelectors.js:5:22)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.559s
Ran all test suites matching "subscriptionSelectors".
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@kunall17 kunall17 changed the title from Fix flow, jest unit test errors to [WIP] Fix flow, jest unit test errors Aug 20, 2017

@@ -28,7 +27,7 @@ class Avatar extends PureComponent {
status?: UserStatus,
realm: string,
shape: 'square' | 'rounded' | 'circle',
onPress: () => void,
onPress?: () => void,

This comment has been minimized.

@borisyankov

borisyankov Aug 20, 2017

Contributor

I agree with literally only this line in the PR 🤣

This comment has been minimized.

@kunall17

kunall17 Aug 20, 2017

Contributor

Sure you will, and me as well because these are temporary fixes and atleast the CI will start working and it has been broken since a week!

@zulipbot zulipbot added reviewed and removed needs review labels Aug 20, 2017

@@ -62,4 +62,4 @@ module.file_ext=.json
module.file_ext=.jsx
[version]
^0.49.1
^0.52.0

This comment has been minimized.

@borisyankov

borisyankov Aug 21, 2017

Contributor

We do not use the latest Flow intentionally (causes issues with other libs). We stick to whatever RN uses.

This comment has been minimized.

@kunall17

kunall17 Aug 21, 2017

Contributor

RN has started to use 0.52 some time back and most probably that is deployed in the 0.47!

This comment has been minimized.

@borisyankov

borisyankov Aug 21, 2017

Contributor

RN 0.47 uses .0.49.1, the next one will use at least 0.53 as this is in their master but is not released yet.

} from '../utils/narrow';
import { shouldBeMuted } from '../utils/message';
import { countUnread } from '../utils/unread';
import { NULL_MESSAGE, NULL_USER, NULL_SUBSCRIPTION } from '../nullObjects';
const specialNarrow = (operand: string): Narrow => [

This comment has been minimized.

@borisyankov

borisyankov Aug 21, 2017

Contributor

Just duplicating code is not a good solution. What happens the next time we got this error? Duplicate more? Slippery slope, my friend.

@@ -43,6 +43,7 @@
"lodash.isequal": "^4.4.0",
"lodash.throttle": "^4.1.1",
"lodash.uniqby": "^4.4.0",
"prop-types": "^15.5.10",

This comment has been minimized.

@borisyankov

borisyankov Aug 21, 2017

Contributor

This does not help, since the warning we get is from libraries that are not updated.

This comment has been minimized.

@kunall17

kunall17 Aug 21, 2017

Contributor

Yeah I was seeing if I can remove the warning which is generated from prop-types! it was left behind and is redundant!

@borisyankov

This comment has been minimized.

Contributor

borisyankov commented Aug 21, 2017

Closing, as tests fixed in another commit.

@zulipbot zulipbot removed the reviewed label Aug 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment