Skip to content
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

[0.54] BlobModule.addNetworkingHandler mock missing in jest setup #18279

Closed
n-kumari opened this issue Mar 8, 2018 · 14 comments
Closed

[0.54] BlobModule.addNetworkingHandler mock missing in jest setup #18279

n-kumari opened this issue Mar 8, 2018 · 14 comments
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Resolution: Locked This issue was locked by the bot.

Comments

@n-kumari
Copy link

n-kumari commented Mar 8, 2018

Upgrading react native to 0.54.0 causes the jest tests to fail with an error
TypeError: BlobModule.addNetworkingHandler is not a function

Environment:

OS: macOS Sierra 10.12.6
Node: 8.9.1
Yarn: 1.3.2
npm: 5.6.0
Watchman: 4.7.0
Xcode: Xcode 9.0 Build version 9A235
Android Studio: Not Found

Packages: (wanted => installed)
react: ^16.3.0-alpha.1 => 16.3.0-alpha.1
react-native: 0.54.0 => 0.54.0

Expected Behavior

The tests should not break on upgrade

Actual Behavior

The test suite fails to run with an error

screen shot 2018-03-08 at 12 12 31 pm

Steps to Reproduce

  • Upgrade react-native to 0.54.0
  • Run jest tests
@n-kumari n-kumari changed the title BlobModule.addNetworkingHandler mock in jest setup BlobModule.addNetworkingHandler mock missing in jest setup Mar 8, 2018
@n-kumari
Copy link
Author

n-kumari commented Mar 8, 2018

This issue can be fixed by adding the addNetworkingHandler mock (in BlobModule) inside the jest setup file.

addNetworkingHandler: jest.fn()

@hramos
Copy link
Contributor

hramos commented Mar 9, 2018

Hmm, the tests for 0.54-stable are green. Can you include more information about the failing test? Which suite was it that failed? Thanks!

@n-kumari
Copy link
Author

n-kumari commented Mar 9, 2018

@FireNoid I think the change should be made in the jest setup file instead of the file node_modules/react-native/Libraries/Blob/BlobManager.js itself.

The setup file is where we should mock the required modules.
So, in node_modules/react-native/jest/setup.js

Change

BlobModule: {
    BLOB_URI_SCHEME: 'content',
    BLOB_URI_HOST: null,
    enableBlobSupport: jest.fn(),
    disableBlobSupport: jest.fn(),
    createFromParts: jest.fn(),
    sendBlob: jest.fn(),
    release: jest.fn(),
  },

to

BlobModule: {
    BLOB_URI_SCHEME: 'content',
    BLOB_URI_HOST: null,
    addNetworkingHandler: jest.fn(),
    enableBlobSupport: jest.fn(),
    disableBlobSupport: jest.fn(),
    createFromParts: jest.fn(),
    sendBlob: jest.fn(),
    release: jest.fn(),
  }

@BachirKhiati
Copy link

BachirKhiati commented Mar 9, 2018

You are right. It didn't work well!
I changed it the same way you suggested. Fixed!
Thank you!

@n-kumari
Copy link
Author

n-kumari commented Mar 9, 2018

@hramos BlobModule .addNetworkingHandler is being used in Libraries/Network/XMLHttpRequest.js but probably does not have a test coverage around it.
Our test suites are designed differently than the ones being used by react-native, so not sure which test suite it is.

Here is the what the failure says

screen shot 2018-03-09 at 9 50 40 am

Let me know if this helps.

@n-kumari n-kumari changed the title BlobModule.addNetworkingHandler mock missing in jest setup [0.54] BlobModule.addNetworkingHandler mock missing in jest setup Mar 9, 2018
@hramos
Copy link
Contributor

hramos commented Mar 9, 2018

@n-kumari ah, good catch. We should update our test suite as well. Thanks for bringing this to our attention!

@hramos hramos added the Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. label Mar 9, 2018
@matt-oakes
Copy link
Contributor

For those wanting a copy paste bit of code, add this to the jest setup file:

import { NativeModules } from "react-native";

NativeModules.BlobModule = {
  ...NativeModules.BlobModule,
  addNetworkingHandler: jest.fn()
};

@react-native-bot react-native-bot added the Platform: macOS Building on macOS. label Mar 20, 2018
@hramos hramos removed the Platform: macOS Building on macOS. label Mar 29, 2018
@gabrielvcbessa
Copy link

@matt-oakes In what file should I paste it? I am getting this error and can't test my project

@BachirKhiati
Copy link

@matt-oakes this is the path: node_modules/react-native/jest/setup.js

@matt-oakes
Copy link
Contributor

@Alu1911 @gabrielvcbessa That would work, but I wouldn't recommend putting it there as you shouldn't edit files inside node_modules if you want reproducible builds. Instead, create your own jest setup file.

campsafari pushed a commit to exozet/react-native that referenced this issue Apr 11, 2018
Summary:
Fixes facebook#18279 by adding the correct methods to the jest mocks setup file.

Test by no longer including the workarounds in [issue comments](facebook#18279 (comment)). A [comment from hramos](facebook#18279 (comment)) mentioned improving the test coverage as well, but I wasn't certain how to achieve that.

[GENERAL] [BUGFIX] [BlobManager] - Fixed the jest mocks to avoid breaking tests
Closes facebook#18718

Differential Revision: D7542458

Pulled By: hramos

fbshipit-source-id: 77c9c7cae77971d62e878c4832b2e1d205131e8f
LukeDurrant pushed a commit to LukeDurrant/react-native that referenced this issue Apr 11, 2018
Summary:
Fixes facebook#18279 by adding the correct methods to the jest mocks setup file.

Test by no longer including the workarounds in [issue comments](facebook#18279 (comment)). A [comment from hramos](facebook#18279 (comment)) mentioned improving the test coverage as well, but I wasn't certain how to achieve that.

[GENERAL] [BUGFIX] [BlobManager] - Fixed the jest mocks to avoid breaking tests
Closes facebook#18718

Differential Revision: D7542458

Pulled By: hramos

fbshipit-source-id: 77c9c7cae77971d62e878c4832b2e1d205131e8f
LukeDurrant pushed a commit to LukeDurrant/react-native that referenced this issue Apr 11, 2018
Summary:
Fixes facebook#18279 by adding the correct methods to the jest mocks setup file.

Test by no longer including the workarounds in [issue comments](facebook#18279 (comment)). A [comment from hramos](facebook#18279 (comment)) mentioned improving the test coverage as well, but I wasn't certain how to achieve that.

[GENERAL] [BUGFIX] [BlobManager] - Fixed the jest mocks to avoid breaking tests
Closes facebook#18718

Differential Revision: D7542458

Pulled By: hramos

fbshipit-source-id: 77c9c7cae77971d62e878c4832b2e1d205131e8f
@davidpaulsson
Copy link

Will a fix for this land in RN soon?

@hramos
Copy link
Contributor

hramos commented May 10, 2018

It landed in commit 43014ea already. That commit has not yet made it to a release, but it should as soon as the next RC is cut.

bunnyc1986 pushed a commit to bunnyc1986/react-native that referenced this issue May 11, 2018
Summary:
Fixes facebook#18279 by adding the correct methods to the jest mocks setup file.

Test by no longer including the workarounds in [issue comments](facebook#18279 (comment)). A [comment from hramos](facebook#18279 (comment)) mentioned improving the test coverage as well, but I wasn't certain how to achieve that.

[GENERAL] [BUGFIX] [BlobManager] - Fixed the jest mocks to avoid breaking tests
Closes facebook#18718

Differential Revision: D7542458

Pulled By: hramos

fbshipit-source-id: 77c9c7cae77971d62e878c4832b2e1d205131e8f
macdoum1 pushed a commit to macdoum1/react-native that referenced this issue Jun 28, 2018
Summary:
Fixes facebook#18279 by adding the correct methods to the jest mocks setup file.

Test by no longer including the workarounds in [issue comments](facebook#18279 (comment)). A [comment from hramos](facebook#18279 (comment)) mentioned improving the test coverage as well, but I wasn't certain how to achieve that.

[GENERAL] [BUGFIX] [BlobManager] - Fixed the jest mocks to avoid breaking tests
Closes facebook#18718

Differential Revision: D7542458

Pulled By: hramos

fbshipit-source-id: 77c9c7cae77971d62e878c4832b2e1d205131e8f
@mseijas
Copy link

mseijas commented Oct 19, 2018

@matt-oakes I'm trying to set this up in the jest config file as you suggested but I'm having trouble with it. Could you share an example of how you've set it up there? Thanks!!

@yogeshchoudhary147
Copy link

screen shot 2018-12-11 at 11 57 28 pm

@n-kumari I am getting this even after trying your method. But package manager show :
BUNDLE [ios, dev] ./index.ios.js ▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓ 100.0% (1/1), done.

This is part of package.json
"devDependencies": {
"babel-eslint": "^7.2.3",
"babel-jest": "18.0.0",
"babel-preset-react-native": "1.9.1",
"eslint": "^3.19.0",
"eslint-plugin-flowtype": "^2.32.1",
"eslint-plugin-react": "^6.10.3",
"jest": "^23.6.0",
"react-test-renderer": "15.4.2"
},
"jest": {
"preset": "react-native"
}

Thanks in advance.

@facebook facebook locked as resolved and limited conversation to collaborators Apr 9, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Apr 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

9 participants