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

Improve unit testing of Widgets #66

Closed
kitsonk opened this issue Mar 10, 2017 · 8 comments
Closed

Improve unit testing of Widgets #66

kitsonk opened this issue Mar 10, 2017 · 8 comments
Assignees
Milestone

Comments

@kitsonk
Copy link
Member

kitsonk commented Mar 10, 2017

Enhancement

We need a good set of unit tests for a widget, that can be run against a virtual DOM (e.g. JSDOM) as well as a unit test in the native DOM without being a functional test. We should only use intended public APIs and generate the events/use synthetic events to drive the tests instead of relying upon driving functionality to provide full unit coverage of the widgets.

@matt-gadd
Copy link
Contributor

matt-gadd commented Mar 24, 2017

As well as this, it might be nice to build into the harness higher level support for asserting v and w calls in an easy manner. At the moment our test assertions kind of straddle the middle ground of asserting VNodes which ideally we don't want to expose to the end user (given we have our own abstraction of a HNode).

If we assert against HNodes and WNodes we can do both shallow rendering (because both node types are lazy), as well as letting the user actually write v and w calls in the assertion block, which I think would be quite nice. For example something like:

const todos = [
    { id: 1, label: 'foo' },
    { id: 2, label: 'bar' }
];

const result = testWrapper(() => w(TodoList, { todos }));

assert.isTrue(result.equals(() => {
    return v('ul', { classes: { 'todo-list': true } }, [
        w(TodoItem, { key: 1, label: 'foo', completed: false }),
        w(TodoItem, { key: 2, label: 'bar', completed: false })
    ]);
}));

@kitsonk
Copy link
Member Author

kitsonk commented Mar 24, 2017

Good thoughts.

I guess though it needs to be permissive. For example if there are additional attributes, like event handlers, the harness won't have access to those references. So either only report missing values or allowing non-primitive equality for certain properties.

The thing too that we would want to avoid with something like that is people just copypasta their render function.

@matt-gadd
Copy link
Contributor

matt-gadd commented Mar 24, 2017

Indeed. It would be similar to sinon's calledWith in that you only test the things that you pass explicitly (so they can be partial). So in this case you wouldn't assert against an event handler in the v call, you'd exercise the behaviour by triggering the event separately.

@rishson
Copy link
Contributor

rishson commented Mar 24, 2017

Is there overlap here with dojo/meta#131?

@kitsonk
Copy link
Member Author

kitsonk commented Mar 24, 2017

That is where the PR will be raised for this. We will reference back to this issue from that repo, but we haven't published @dojo/intern-helper yet, so dojo/meta#131 needs to stay open. We could in theory move this issue to dojo/intern-helper, but we can reference issues across repos.

@rishson
Copy link
Contributor

rishson commented Mar 24, 2017

👍 gotcha

@dylans dylans added this to the 2017.04 milestone Mar 30, 2017
kitsonk added a commit to dojo/test-extras that referenced this issue Apr 10, 2017
@kitsonk kitsonk reopened this Apr 10, 2017
@kitsonk kitsonk mentioned this issue Apr 10, 2017
3 tasks
mwistrand pushed a commit to mwistrand/widgets that referenced this issue Apr 24, 2017
@dylans dylans modified the milestones: 2017.04, 2017.05 Apr 29, 2017
@eheasley eheasley added beta3 and removed beta2 labels Jun 6, 2017
@eheasley eheasley modified the milestones: 2017.05, 2017.06 Jun 6, 2017
@agubler
Copy link
Member

agubler commented Jun 23, 2017

@kitsonk can this be closed now given the test-extras and harness work?

@kitsonk
Copy link
Member Author

kitsonk commented Jun 23, 2017

Agreed.

@kitsonk kitsonk closed this as completed Jun 23, 2017
nicknisi pushed a commit to dojo/framework that referenced this issue Jul 16, 2018
@tomdye tomdye mentioned this issue Jan 30, 2020
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants