-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
fix: fix query regression and unit testing improvements #148
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,17 @@ | ||
/** | ||
* @deprecated Deprecated in favour of Spectator query methods and Spectator custom matchers | ||
* | ||
* To be removed in v4 | ||
*/ | ||
export function query(selector: string, parent?) { | ||
return (parent || document).querySelector(selector); | ||
} | ||
|
||
/** | ||
* @deprecated Deprecated in favour of Spectator query methods and Spectator custom matchers | ||
* | ||
* To be removed in v4 | ||
*/ | ||
export function queryAll(selector: string, parent?) { | ||
return (parent || document).querySelectorAll(selector); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,6 @@ import { createMouseEvent } from './event-objects'; | |
import { typeInElement } from './type-in-element'; | ||
import { patchElementFocus } from './element-focus'; | ||
import { Observable } from 'rxjs'; | ||
import { SpectatorDebugElementNotFoundError } from './errors'; | ||
import { SpyObject } from './mock'; | ||
import { DOMSelector } from './dom-selectors'; | ||
import { Token } from './token'; | ||
|
@@ -67,15 +66,7 @@ export class Spectator<C> { | |
query<R>(directive: Type<R>): R; | ||
query<R>(directiveOrSelector: Type<any> | string, options: { read: Token<R> }): R; | ||
query<R>(directiveOrSelector: Type<any> | DOMSelector | string, options: { read: Token<R> } = { read: undefined }): R { | ||
try { | ||
return _getChild<R>(this.debugElement)(directiveOrSelector, options); | ||
} catch (err) { | ||
if (err instanceof SpectatorDebugElementNotFoundError) { | ||
return null; | ||
} else { | ||
throw err; | ||
} | ||
} | ||
return _getChild<R>(this.debugElement)(directiveOrSelector, options); | ||
} | ||
|
||
queryAll<R extends Element[]>(selector: string | DOMSelector): R; | ||
|
@@ -290,13 +281,7 @@ export class Spectator<C> { | |
*/ | ||
export function _getChild<R>(debugElementRoot: DebugElement) { | ||
return function(directiveOrSelector: Type<R> | DOMSelector | string, options: { read } = { read: undefined }): R { | ||
const [child] = _getChildren<R>(debugElementRoot)(directiveOrSelector, options); | ||
|
||
if (!child) { | ||
throw new SpectatorDebugElementNotFoundError(`Cannot find a debug element for ${directiveOrSelector}`); | ||
} | ||
|
||
return child; | ||
return _getChildren<R>(debugElementRoot)(directiveOrSelector, options)[0] || null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should at least add console info so that users will know what is missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that was before also not the case, right? Because the error was catched? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, sorry, I see. I'll fix it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, well, it is still thrown right? Because it is not catched? |
||
}; | ||
} | ||
|
||
|
@@ -312,24 +297,20 @@ export function _getChildren<R>(debugElementRoot: DebugElement) { | |
return directiveOrSelector.execute(debugElementRoot.nativeElement) as any[]; | ||
} | ||
|
||
let debugElement: DebugElement[]; | ||
let debugElements: DebugElement[]; | ||
|
||
if (typeof directiveOrSelector === 'string') { | ||
debugElement = debugElementRoot.queryAll(By.css(directiveOrSelector)); | ||
debugElements = debugElementRoot.queryAll(By.css(directiveOrSelector)); | ||
} else { | ||
debugElement = debugElementRoot.queryAll(By.directive(directiveOrSelector)); | ||
} | ||
|
||
if (!debugElement) { | ||
throw new SpectatorDebugElementNotFoundError(`Cannot find a debug element for ${directiveOrSelector}`); | ||
debugElements = debugElementRoot.queryAll(By.directive(directiveOrSelector)); | ||
} | ||
|
||
if (options.read) { | ||
return debugElement.map(debug => debug.injector.get(options.read)); | ||
return debugElements.map(debug => debug.injector.get(options.read)); | ||
} else if (typeof directiveOrSelector === 'string') { | ||
return debugElement.map(debug => debug.nativeElement); | ||
return debugElements.map(debug => debug.nativeElement); | ||
} else { | ||
return debugElement.map(debug => debug.componentInstance); | ||
return debugElements.map(debug => debug.componentInstance); | ||
} | ||
}; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import { Directive, EventEmitter, Type } from '@angular/core'; | |
* MockDirective({ selector: 'some-directive' }); | ||
* MockDirective({ selector: 'some-directive', inputs: ['some-input', 'some-other-input'] }); | ||
* | ||
* @deprecated Deprecated in favour of the `ng-mocks` implementation of `MockDirective`. To be be removed in the next major version. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed in v4 PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 |
||
*/ | ||
export function MockDirective(options: Directive & { identifier?: Type<any> }): Directive { | ||
const metadata: Directive & { identifier?: Type<any> } = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
{ | ||
"extends": "../tsconfig.json", | ||
"compilerOptions": { | ||
"outDir": "../out-tsc/spec", | ||
"module": "commonjs", | ||
"baseUrl": "./", | ||
"types": [ | ||
"jest", | ||
"node" | ||
], | ||
"paths": { | ||
"@netbasal/spectator": [ | ||
"../projects/spectator/src/lib" | ||
], | ||
"@netbasal/spectator/jest": [ | ||
"../projects/spectator/jest/src" | ||
] | ||
} | ||
}, | ||
"files": [ | ||
"test.ts", | ||
"polyfills.ts" | ||
], | ||
"include": [ | ||
"**/*.jest.ts", | ||
"**/*.d.ts", | ||
"../projects/spectator/src/lib/matchers-types.d.ts" | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you removed the watch option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can run both
test
andtest:jest
in watch mode with--watch
so it felt a bit unneeded. ;-)