Skip to content

feat: Store selector in Wrapper.find() / .findAll() #1248

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
merged 5 commits into from
Dec 18, 2019
Merged

feat: Store selector in Wrapper.find() / .findAll() #1248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f156698
chore: Rename selector to selectorString in ErrorWrapper
winniehell May 30, 2019
46f884e
chore: Build selector string in ErrorWrapper
winniehell May 30, 2019
4ec04f0
feat: Add selector property to BaseWrapper interface
winniehell May 31, 2019
aab0bf5
feat: Store selector in Wrapper.find() / .findAll()
winniehell May 31, 2019
ff4e0fc
chore: Remove selectorString from ErrorWrapper
winniehell May 31, 2019
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
4 changes: 4 additions & 0 deletions docs/api/wrapper/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ A `Wrapper` is an object that contains a mounted component or vnode and methods

`Boolean` (read-only): True if component is attached to document when rendered.

### `selector`

`Selector`: the selector that was used by [`find()`](./find.md) or [`findAll()`](./findAll.md) to create this wrapper

## Methods

!!!include(docs/api/wrapper/attributes.md)!!!
Expand Down
1 change: 1 addition & 0 deletions flow/wrapper.flow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ declare interface BaseWrapper {
name(): string | void;
props(key?: string): { [name: string]: any } | any | void;
text(): string | void;
selector: Selector | void;
setData(data: Object): void;
setMethods(methods: Object): void;
setValue(value: any): void;
Expand Down
142 changes: 109 additions & 33 deletions packages/test-utils/src/error-wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,73 @@
// @flow

import { throwError } from 'shared/util'
import { REF_SELECTOR } from 'shared/consts'
import { getSelectorType } from './get-selector'

const buildSelectorString = (selector: Selector) => {
if (getSelectorType(selector) === REF_SELECTOR) {
return `ref="${selector.value.ref}"`
}

if (typeof selector === 'string') {
return selector
}

return 'Component'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that we could use the component name at least in some cases here. However I'd like to keep that as a follow-up.

}

export default class ErrorWrapper implements BaseWrapper {
selector: string
selector: Selector

constructor(selector: string) {
constructor(selector: Selector) {
this.selector = selector
}

at(): void {
throwError(
`find did not return ${this.selector}, cannot call at() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call at() on empty Wrapper`
)
}

attributes(): void {
throwError(
`find did not return ${this.selector}, cannot call attributes() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call attributes() on empty Wrapper`
)
}

classes(): void {
throwError(
`find did not return ${this.selector}, cannot call classes() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call classes() on empty Wrapper`
)
}

contains(): void {
throwError(
`find did not return ${this.selector}, cannot call contains() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call contains() on empty Wrapper`
)
}

emitted(): void {
throwError(
`find did not return ${this.selector}, cannot call emitted() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call emitted() on empty Wrapper`
)
}

emittedByOrder(): void {
throwError(
`find did not return ${this.selector}, cannot call emittedByOrder() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call emittedByOrder() on empty Wrapper`
)
}

Expand All @@ -51,151 +77,201 @@ export default class ErrorWrapper implements BaseWrapper {

filter(): void {
throwError(
`find did not return ${this.selector}, cannot call filter() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call filter() on empty Wrapper`
)
}

visible(): void {
throwError(
`find did not return ${this.selector}, cannot call visible() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call visible() on empty Wrapper`
)
}

hasAttribute(): void {
throwError(
`find did not return ${this.selector}, cannot call hasAttribute() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call hasAttribute() on empty Wrapper`
)
}

hasClass(): void {
throwError(
`find did not return ${this.selector}, cannot call hasClass() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call hasClass() on empty Wrapper`
)
}

hasProp(): void {
throwError(
`find did not return ${this.selector}, cannot call hasProp() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call hasProp() on empty Wrapper`
)
}

hasStyle(): void {
throwError(
`find did not return ${this.selector}, cannot call hasStyle() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call hasStyle() on empty Wrapper`
)
}

findAll(): void {
throwError(
`find did not return ${this.selector}, cannot call findAll() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call findAll() on empty Wrapper`
)
}

find(): void {
throwError(
`find did not return ${this.selector}, cannot call find() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call find() on empty Wrapper`
)
}

html(): void {
throwError(
`find did not return ${this.selector}, cannot call html() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call html() on empty Wrapper`
)
}

is(): void {
throwError(
`find did not return ${this.selector}, cannot call is() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call is() on empty Wrapper`
)
}

isEmpty(): void {
throwError(
`find did not return ${this.selector}, cannot call isEmpty() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call isEmpty() on empty Wrapper`
)
}

isVisible(): void {
throwError(
`find did not return ${this.selector}, cannot call isVisible() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call isVisible() on empty Wrapper`
)
}

isVueInstance(): void {
throwError(
`find did not return ${this.selector}, cannot call isVueInstance() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call isVueInstance() on empty Wrapper`
)
}

name(): void {
throwError(
`find did not return ${this.selector}, cannot call name() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call name() on empty Wrapper`
)
}

props(): void {
throwError(
`find did not return ${this.selector}, cannot call props() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call props() on empty Wrapper`
)
}

text(): void {
throwError(
`find did not return ${this.selector}, cannot call text() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call text() on empty Wrapper`
)
}

setComputed(): void {
throwError(
`find did not return ${this.selector}, cannot call setComputed() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setComputed() on empty Wrapper`
)
}

setData(): void {
throwError(
`find did not return ${this.selector}, cannot call setData() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setData() on empty Wrapper`
)
}

setMethods(): void {
throwError(
`find did not return ${this.selector}, cannot call setMethods() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setMethods() on empty Wrapper`
)
}

setProps(): void {
throwError(
`find did not return ${this.selector}, cannot call setProps() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setProps() on empty Wrapper`
)
}

setValue(): void {
throwError(
`find did not return ${this.selector}, cannot call setValue() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setValue() on empty Wrapper`
)
}

setChecked(): void {
throwError(
`find did not return ${this.selector}, cannot call setChecked() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setChecked() on empty Wrapper`
)
}

setSelected(): void {
throwError(
`find did not return ${this.selector}, cannot call setSelected() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call setSelected() on empty Wrapper`
)
}

trigger(): void {
throwError(
`find did not return ${this.selector}, cannot call trigger() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call trigger() on empty Wrapper`
)
}

destroy(): void {
throwError(
`find did not return ${this.selector}, cannot call destroy() on empty Wrapper`
`find did not return ${buildSelectorString(
this.selector
)}, cannot call destroy() on empty Wrapper`
)
}
}
2 changes: 1 addition & 1 deletion packages/test-utils/src/get-selector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
INVALID_SELECTOR
} from 'shared/consts'

function getSelectorType(selector: Selector): string {
export function getSelectorType(selector: Selector): string {
if (isDomSelector(selector)) return DOM_SELECTOR
if (isVueComponent(selector)) return COMPONENT_SELECTOR
if (isNameSelector(selector)) return NAME_SELECTOR
Expand Down
1 change: 1 addition & 0 deletions packages/test-utils/src/wrapper-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { throwError } from 'shared/util'
export default class WrapperArray implements BaseWrapper {
+wrappers: Array<Wrapper | VueWrapper>
+length: number
selector: Selector | void

constructor(wrappers: Array<Wrapper | VueWrapper>) {
const length = wrappers.length
Expand Down
21 changes: 12 additions & 9 deletions packages/test-utils/src/wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default class Wrapper implements BaseWrapper {
+options: WrapperOptions
isFunctionalComponent: boolean
rootNode: VNode | Element
selector: Selector | void

constructor(
node: VNode | Element,
Expand Down Expand Up @@ -193,15 +194,12 @@ export default class Wrapper implements BaseWrapper {
const node = find(this.rootNode, this.vm, selector)[0]

if (!node) {
if (selector.type === REF_SELECTOR) {
return new ErrorWrapper(`ref="${selector.value.ref}"`)
}
return new ErrorWrapper(
typeof selector.value === 'string' ? selector.value : 'Component'
)
return new ErrorWrapper(rawSelector)
}

return createWrapper(node, this.options)
const wrapper = createWrapper(node, this.options)
wrapper.selector = rawSelector
return wrapper
}

/**
Expand All @@ -214,9 +212,14 @@ export default class Wrapper implements BaseWrapper {
const wrappers = nodes.map(node => {
// Using CSS Selector, returns a VueWrapper instance if the root element
// binds a Vue instance.
return createWrapper(node, this.options)
const wrapper = createWrapper(node, this.options)
wrapper.selector = rawSelector
return wrapper
})
return new WrapperArray(wrappers)

const wrapperArray = new WrapperArray(wrappers)
wrapperArray.selector = rawSelector
return wrapperArray
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/test-utils/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ interface BaseWrapper {

trigger (eventName: string, options?: object): void
destroy (): void
selector: Selector | void
Copy link
Member

Choose a reason for hiding this comment

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

This isn't actually a Selector type, it's an object with a value and type: https://github.com/vuejs/vue-test-utils/blob/dev/packages/test-utils/src/get-selector.js#L38

Because the library isn't written in TS, we have tests (https://github.com/vuejs/vue-test-utils/blob/dev/packages/test-utils/types/test/wrapper.ts) that access and set the wrapper options using the types. Can you write a test that accesses selector.value and selector.type and update this types file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't actually a Selector type

@eddyerburgh Thank you for catching this! 👍

It actually is of type Selector for the ErrorWrapper:

vue-test-utils/packages/test-utils/src/error-wrapper.js

Line 22 in 70ac83a

this.selector = selector

I will update the other places to use rawSelector (which is of type Selector), too, because that prevents us from defining a new type and consumers of the API don't need to do selector.value to get the selector they provided in the first place.

we have tests (https://github.com/vuejs/vue-test-utils/blob/dev/packages/test-utils/types/test/wrapper.ts)

Sure, I can add tests there! 👍

Right now I'm a little confused by the structure of that file though because it has neither test cases in the usual style (no it('something happens', () => { ... }) / test(...) blocks) nor assertions (only implicit ones). Could you provide me with an example of how such a test would look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually is of type Selector for the ErrorWrapper:

Ah, it turns out this was wrong actually—but the Selector type is currently any:

vue-test-utils/flow/wrapper.flow.js

Line 6 in 70ac83a

declare type Selector = any

I will adjust the places that create ErrorWrapper instances to use the rawSelector as well then.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, the test script just compiles the file to make sure there aren't any compiler errors when using the Vue Test Utils API, caused by incorrect typing.

A test would look something like this:

let s: string = wrapper.selector.value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! 🙇‍♂️ I'll add the tests and ping you again once they are ready.

}

export interface Wrapper<V extends Vue | null> extends BaseWrapper {
Expand Down
Loading