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

Rewrite of the test harness #535

Closed
matt-gadd opened this issue Sep 14, 2019 · 2 comments · Fixed by #710
Closed

Rewrite of the test harness #535

matt-gadd opened this issue Sep 14, 2019 · 2 comments · Fixed by #710
Labels
area: testing Testing next Issue/Pull Request for the next major version

Comments

@matt-gadd
Copy link
Contributor

Enhancement
The testing harness is one of the oldest parts of the codebase, and since then a number of the new patterns have emerged such as render props that it doesn't deal very well with. In Dojo 7 we'd like to revisit the harness and see how we can improve the ergonomics as well as offering more options in terms of rendering.

Some goals:

  • Expose less of the underlying DNodes to the end user, in favour of providing better mechanisms to trigger and expect assertions with.
  • Support the existing virtual rendering (no vdom at all) as well as shallow, and full rendering to actual dom.
  • First class support for render props
  • Better debugging output when assertions fail
@matt-gadd matt-gadd added next Issue/Pull Request for the next major version area: testing Testing labels Sep 14, 2019
@agubler agubler mentioned this issue Sep 14, 2019
@JamesLMilner
Copy link
Contributor

JamesLMilner commented Nov 29, 2019

Some thoughts about areas we might be able to improve the ergonomics of the test harness:

Render Props

If we had a list render prop we might test it like so:

h.expect(listAssertion, () =>
	h.trigger('@list', (node: any) => node.properties.renderList, listItem)
);

Here we lose the type safety because node is cast to any to silence type errors. This extends further when you have to test a function property in the the renderProp's resulting element:

h.trigger('@list', (node: any) => node.properties.renderProp.properties.onClick);

This complexity grows with nesting; if the result render prop returns multiple elements/widgets and then that element has a property which is an array of items which has a function in it you want to trigger it might look like this:

h.trigger('@list', (node: any) => node.properties.renderProp[1].properties.items[2].doSomething);

Taking the select approach is also somewhat verbose and not type safe:

h.trigger(
	'@list',
	(node) =>
		(select(
			'[name=button]',
			(node as any).properties.renderProp()
		)[0] as any).properties.onClick
);

Deep Property Setting

Updating a property in an object property feels a little verbose, it might be nice to have a convenience function here:

baseTemplate.setProperty('@someKey', 'inputProperties', {
	...baseTemplate.getProperty('@someKey', 'inputProperties'),
	value: 'updated'
})

@JamesLMilner
Copy link
Contributor

JamesLMilner commented Dec 12, 2019

Another case with render props can be seen by the following example; imagine we have the following widget:

<SomeWidget
    key="widget"
    button={
        <Button>
            <span key="title" classes={[themedCss.title, commonThemedCss.fontHeader2]}>{title}</span>
            <div key="icons" classes={themedCss.icons}>
                <Icon icon={"chevron"} />
            </div>
        </Button>
    }
</SomeWidget>

Currently if we wanted to update @title in the assertion template we'd have to do something like the following:

h.expect(baseAssertion.setProperty(
	'@menu',
	'button',
	<Button>
		<span key="title" classes={[css.title, common.fontHeader2]}>A new string</span>
		<div key="icons" classes={css.icons}>
			<Icon icon={"chevron"} />
		</div>
	</Button>
));

As opposed to using replaceChildren('@title', 'A new string') in this situation.

@agubler agubler mentioned this issue Mar 31, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: testing Testing next Issue/Pull Request for the next major version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants