Skip to content

implement spyOnProperty method #5107

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.

Sign up for GitHub

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

Merged
cpojer merged 18 commits into from
Jan 8, 2018
Merged

implement spyOnProperty method #5107

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1a3b82a
feat: implement spyOnProperty method, fixes #5106
phra Dec 18, 2017
4cfb737
style: fix indentation of mock, runtime, jasmine2
phra Dec 18, 2017
65d884d
test: fix failing tests for spyOnProperty
phra Dec 18, 2017
a4a6313
test: implement tests for #1214
phra Dec 25, 2017
d26337c
style: fix eslint errors
phra Dec 25, 2017
7c4844d
refactor: proxy spyOnProperty call behind spyOn
phra Jan 4, 2018
afb0e96
refactor: remove useless console.log
phra Jan 4, 2018
05a303d
style: fix eslint errors
phra Jan 4, 2018
fc0de22
types: remove declaration of spyOnProperty
phra Jan 4, 2018
0a5e083
docs: add documentation for accessType argument of spyOn
phra Jan 5, 2018
c0894d9
docs: fix typo in spyOn docs
phra Jan 6, 2018
be83146
test(spyOn): fix typo in should throw on invalid input
phra Jan 6, 2018
e5aa9a6
test(spyOn): add tests for setters
phra Jan 6, 2018
26b5ddb
docs(spyOn): add example for spying on setters
phra Jan 6, 2018
95f0ab9
style: fix eslint errors
phra Jan 6, 2018
bd44ca5
refactor: format error messages with getErrorMsg()
phra Jan 8, 2018
37093f6
style: fix eslint errors
phra Jan 8, 2018
224eb25
revert: restore snapshotState.getUncheckedKeys()
phra Jan 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
test: fix failing tests for spyOnProperty
  • Loading branch information
@phra
phra committed Dec 19, 2017
commit 65d884d49b450e4dabf959577ed60f2ed751ffeb
2 changes: 1 addition & 1 deletion flow-typed/npm/jest_v21.x.x.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ declare var expect: {
// TODO handle return type
// http://jasmine.github.io/2.4/introduction.html#section-Spies
declare function spyOn(value: mixed, method: string): Object;
declare function spyOnProperty(value: mixed, propertyName: string, accessType: 'get' | 'set'): Object;
declare function spyOnProperty(value: mixed, propertyName: string, accessType: string): Object;

/** Holds all functions related to manipulating test runner */
declare var jest: JestObjectType;
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-jasmine2/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const addSnapshotData = (results, snapshotState) => {
});

const uncheckedCount = snapshotState.getUncheckedCount();
let uncheckedKeys
let uncheckedKeys;

if (uncheckedCount) {
uncheckedKeys = snapshotState.getUncheckedKeys();
Copy link
Member

@cpojer cpojer Jan 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this changed? This seems unrelated.

Copy link
Contributor Author

@phra phra Jan 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i got runtime errors when developing this feature when uncheckedCount is falsy. do you think that this change is not required?

Copy link
Member

@cpojer cpojer Jan 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be changed in a separate commit, with a test that is fixed by your change. Please revert it from this PR and send one for this issues specifically. Thank you.

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-jasmine2/src/jasmine/jasmine_light.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ exports.interface = function(jasmine: Jasmine, env: any) {
return env.spyOn(obj, methodName);
},

spyOnProperty: function (obj: Object, methodName: string, accessType = 'get') {
spyOnProperty(obj: Object, methodName: string, accessType = 'get') {
return env.spyOnProperty(obj, methodName, accessType);
},

Expand Down
18 changes: 12 additions & 6 deletions packages/jest-jasmine2/src/jasmine/spy_registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ export default function SpyRegistry(options: Object) {

this.spyOnProperty = function(obj, propertyName, accessType = 'get') {
if (!obj) {
throw new Error('spyOn could not find an object to spy upon for ' + propertyName + '');
throw new Error(
'spyOn could not find an object to spy upon for ' + propertyName + '',
Copy link
Member

@cpojer cpojer Jan 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an empty string at the end. Can we end the sentence with a .? Also, can we prefix the error with "Jest: "?

Copy link
Contributor Author

@phra phra Jan 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will do it.

Copy link
Contributor Author

@phra phra Jan 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpojer in the end i've used getErrorMsg() function as in the spyOn function. is that better than manual format the string, isn'it?
see bd44ca5

);
}

if (!propertyName) {
Expand All @@ -154,7 +156,9 @@ export default function SpyRegistry(options: Object) {
}

if (!descriptor[accessType]) {
throw new Error('Property ' + propertyName + ' does not have access type ' + accessType);
throw new Error(
'Property ' + propertyName + ' does not have access type ' + accessType,
);
}

if (obj[propertyName] && isSpy(obj[propertyName])) {
Expand All @@ -172,20 +176,22 @@ export default function SpyRegistry(options: Object) {
let restoreStrategy;

if (Object.prototype.hasOwnProperty.call(obj, propertyName)) {
restoreStrategy = function () {
restoreStrategy = function() {
Object.defineProperty(obj, propertyName, originalDescriptor);
};
} else {
restoreStrategy = function () {
restoreStrategy = function() {
delete obj[propertyName];
};
}

currentSpies().push({
restoreObjectToOriginalState: restoreStrategy
restoreObjectToOriginalState: restoreStrategy,
});

const spiedDescriptor = Object.assign({}, descriptor, { [accessType]: spiedProperty })
const spiedDescriptor = Object.assign({}, descriptor, {
[accessType]: spiedProperty,
});

Object.defineProperty(obj, propertyName, spiedDescriptor);

Expand Down
29 changes: 14 additions & 15 deletions packages/jest-mock/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,27 +691,24 @@ class ModuleMockerClass {
return object[methodName];
}

spyOnProperty(object: any, propertyName: any, accessType = 'get'): any {
if (typeof object !== 'object' && typeof object !== 'function') {
spyOnProperty(obj: any, propertyName: any, accessType: string = 'get'): any {
if (typeof obj !== 'object' && typeof obj !== 'function') {
throw new Error(
'Cannot spyOn on a primitive value; ' + this._typeOf(object) + ' given',
'Cannot spyOn on a primitive value; ' + this._typeOf(obj) + ' given',
);
}

if (!obj) {
throw new Error('spyOn could not find an object to spy upon for ' + propertyName + '');
throw new Error(
'spyOn could not find an object to spy upon for ' + propertyName + '',
);
}

if (!propertyName) {
throw new Error('No property name supplied');
}

let descriptor;
try {
descriptor = Object.getOwnPropertyDescriptor(obj, propertyName);
} catch (e) {
// IE 8 doesn't support `definePropery` on non-DOM nodes
}
const descriptor = Object.getOwnPropertyDescriptor(obj, propertyName);

if (!descriptor) {
throw new Error(propertyName + ' property does not exist');
Expand All @@ -722,27 +719,29 @@ class ModuleMockerClass {
}

if (!descriptor[accessType]) {
throw new Error('Property ' + propertyName + ' does not have access type ' + accessType);
throw new Error(
'Property ' + propertyName + ' does not have access type ' + accessType,
);
}

const original = descriptor[accessType]
const original = descriptor[accessType];

if (!this.isMockFunction(original)) {
if (typeof original !== 'function') {
throw new Error(
'Cannot spy the ' +
methodName +
propertyName +
' property because it is not a function; ' +
this._typeOf(original) +
' given instead',
);
}

descriptor[accessType] = this._makeComponent({ type: 'function' }, () => {
descriptor[accessType] = this._makeComponent({type: 'function'}, () => {
descriptor[accessType] = original;
});

descriptor[accessType].mockImplementation(function () {
descriptor[accessType].mockImplementation(function() {
return original.apply(this, arguments);
});
}
Expand Down
4 changes: 3 additions & 1 deletion packages/jest-runtime/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,9 @@ class Runtime {
};
const fn = this._moduleMocker.fn.bind(this._moduleMocker);
const spyOn = this._moduleMocker.spyOn.bind(this._moduleMocker);
const spyOnProperty = this._moduleMocker.spyOnProperty.bind(this._moduleMocker);
const spyOnProperty = this._moduleMocker.spyOnProperty.bind(
this._moduleMocker,
);

const setTimeout = (timeout: number) => {
this._environment.global.jasmine
Expand Down
6 changes: 5 additions & 1 deletion types/Jest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ export type Jest = {|
setMock(moduleName: string, moduleExports: any): Jest,
setTimeout(timeout: number): Jest,
spyOn(object: Object, methodName: string): JestMockFn,
spyOnProperty(object: Object, methodName: string, accessType: string): JestMockFn,
spyOnProperty(
object: Object,
methodName: string,
accessType: string,
): JestMockFn,
unmock(moduleName: string): Jest,
useFakeTimers(): Jest,
useRealTimers(): Jest,
Expand Down