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

tests - worth updating to follow the dojo conversion #15

Closed
lbod opened this issue Jan 15, 2014 · 11 comments
Closed

tests - worth updating to follow the dojo conversion #15

lbod opened this issue Jan 15, 2014 · 11 comments

Comments

@lbod
Copy link
Member

lbod commented Jan 15, 2014

It'd be worth following what @bryanforbes has been doing at https://github.com/bryanforbes/dojo/tree/intern-conversion/tests-intern and using a similar grunt and test file structure (maybe even the services JSGI 'middleware')

I don't mind taking this on

@bryanforbes
Copy link
Contributor

To be honest, there shouldn't be a need for services for this project. It would, however, be useful to set up either a mock async store or a dojo/request provider to simulate a backend. Since dojo/store and dojo/request should be fully tested, widgets should only need to interface with those APIs.

@lbod
Copy link
Member Author

lbod commented Jan 15, 2014

About services, agree and disagree, the problem is intern doesn't do stubs/mocking etc yet so you need to build something yourself.
Ignoring services, I think the structure and grunt you're using for tests is a good convention to follow

@wkeese
Copy link
Member

wkeese commented Jan 15, 2014

I too definitely want the shell scripts (runsauce.sh) converted into grunt tasks (for both delite and deliteful), please take that on. I thought the test directory structure for deliteful just got updated to match dojo, but maybe not.

As for services, I agree eventually we will want them (but they don't need to be part of this ticket). I think @bryanforbes is forgetting about widgets like ComboBox, and the need to test what happens when the user types more characters before an XHR completes. See https://github.com/dojo/dijit/blob/master/tests/form/robot/_autoComplete_a11y.html#L691. Of course we can still use PHP to test that, but going full javascript would be a bit better.

@bryanforbes
Copy link
Contributor

@lbod There is no need for intern to "do stubs/mocking" since Dojo provides a store API to program to and a request API that can be mocked.

@wkeese I'm not forgetting about that use case at all. In fact, by using a dojo/request provider, you can mock a request that can be cancelled, is delayed, etc. This could even be done with a testing store. All this test you've pointed me to is testing is that when there is a delay between the request and the resolution of the request, the widget will handle it correctly. There's no reason to go over the wire to test that when JavaScript provides us a way to do that in setTimeout.

@cjolif
Copy link
Contributor

cjolif commented Jan 15, 2014

Yes the deliteful structure is probably very similar to dojo now. I guess what is missing is the delite one.

@wkeese
Copy link
Member

wkeese commented Jan 15, 2014

PS: See ibm-js/sdk#2 for our discussion of the directory structure

@bryanforbes: OK understood about dojo/request, although I can't guarantee that we will end up running on top of dojo core. But regardless, I guess you are right that a delayed XHR can be simulated by stub code running in the browser. I suppose that's better than going over the wire since the timing is more consistent.

@lbod
Copy link
Member Author

lbod commented Jan 15, 2014

@bryanforbes this is a completely separate discussion but a dojo/request provider couldn't mock 401 headers from what I saw. The services thing was just a side comment
I guess I'm saying the structure of the dojo tests, the grunt tasks, making sure tests are using chai (or whatever else) properly and testing using the appropriate api method is important as a convention.
Also, I hadn't seen the rationale for appending a . to a public api method on a test name explained, conventions like that are good to follow across projects

@bryanforbes
Copy link
Contributor

@lbod All the provider needs to do is define a getHeaders() function on the response object that returns the right headers and set the status property on the response object. The API was designed from the beginning to be able to be mocked.

The naming convention we've used for the intern conversion for Dojo is to name the test suite after the module name. Test names should either be what feature a test is testing or the name of the function or method being tested. If the test is testing a function, prepend a .. If the test is testing a method, prepend a #. This follows the common JavaScript naming conventions of object.function() and Class#method().

@bryanforbes
Copy link
Contributor

And offline discussion with @lbod and a fiddle later and I've come up with an example of mocking an asynchronous request using the dojo/request API: http://jsfiddle.net/bryanforbes/2AjPQ/

This was quick and dirty, so YMMV if you use this as is.

@wkeese
Copy link
Member

wkeese commented Jan 29, 2014

Conversion from shell scripts (runsauce.sh etc.) to using grunt is blocked by theintern/intern#139.

wkeese added a commit that referenced this issue Jan 30, 2014
Fix excludeInstrumentation setting.

Note: directory structure / file names not updated to match dojo.

Refs #15, ibm-js/sdk#2 sort-of.
wkeese pushed a commit to wkeese/delite that referenced this issue Jan 31, 2014
@lbod
Copy link
Member Author

lbod commented Feb 14, 2014

theintern/intern@01da2eb has fixed theintern/intern#139 so I've raised a separate issue for grunt tasks in ibm-js/delite#110

@lbod lbod closed this as completed Feb 14, 2014
wkeese pushed a commit to ibm-js/delite that referenced this issue Oct 23, 2014
wkeese added a commit that referenced this issue Feb 6, 2015
Fix excludeInstrumentation setting.

Note: directory structure / file names not updated to match dojo.

Refs #15, ibm-js/sdk#2 sort-of.
wkeese added a commit to wkeese/deliteful that referenced this issue Mar 12, 2015
Fix excludeInstrumentation setting.

Note: directory structure / file names not updated to match dojo.

Refs ibm-js#15, ibm-js/sdk#2 sort-of.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants