[GitHub] cordova-lib pull request #568: Reorganized unit test directory structure + u...

[filmaj]

Github user filmaj closed the pull request at:

https://github.com/apache/cordova-lib/pull/568


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #568: Reorganized unit test directory structure + u...

[filmaj]

GitHub user filmaj opened a pull request:

https://github.com/apache/cordova-lib/pull/568

Reorganized unit test directory structure + updated npm task names

### What does this PR do?

This is a proof-of-concept pull request mainly introduction file 
organizational changes. Please treat this PR as a discussion point, not as 
something that needs to be mergeable right away. I wanted to get the feedback 
of the community and PMC on this change before I went ahead with anything.

Ping @purplecabbage, @stevengill, @shazron, @mwbrooks, @devgeeks, 
@surajpindoria, @imhotep, @dpogue, @kerrishotts, @matrosov-nikita, @goya, 
@jcesarmobile, @macdonst, @infil00p. If I missed anyone, please pull them in 
too ;)

The single biggest change, and takeaway, from this PR is that the unit 
tests under `spec-cordova/` and `spec-plugman/` are consolidated into a single 
top-level `spec/` directory. The idea behind this change (and hopefully future 
to come changes) is that there will exist one unit test spec file per 
javascript source file as a rough rule-of-thumb. Further, the test directory 
structure should mirror, as closely as possible, the source directory 
structure. I feel that that is an implied expectation when it comes to unit 
test file structures. I think possibly one reason why so many integration tests 
exist in this repository is that that structure right now is not honoured. So, 
when contributions come in, the typical expectation of "I made a change in 
src/foo.js, but I don't see a unit test file under spec/foo.spec.js" is broken, 
and people feel they have to write integration tests to get the kind of test 
coverage their contribution requires. Hopefully we can break that pattern here 
an
 d continue to march towards a healthier and faster test suite.

Other than the directory changes, a summary of smaller but also relevant 
changes:

 - consolidated the differing spec-plugman and spec-cordova jasmine 
configuration files
 - consolidated all the helper modules (there are three!?) under the 
top-level `spec/` directory: `helper.js`, `helpers.js` and `common.js`. 
Combining those into one test-helper file is left as an exercise for a future 
pull request :)
 - changed `package.json` `npm run` tasks to better reflect the purpose of 
the task. In particular, changed `npm run jasmine` to `npm run unit-tests`. I 
also thought of changing the `jshint` task to be `linter`, but then decided 
against it. I am open to hearing opinions on this.
 - related to the last point, now running `npm test` runs the integration 
tests _as well_. They currently take around 400 seconds to run on my machine, 
but without them right now we cannot guarantee we will have regressions. I am 
hoping that as we rewrite more and more integration tests into smaller, faster, 
more focussed unit tests, the integration test times will continue to drop.
 - made some README changes to reflect the change in `npm` task names, and 
also fixed the link to the issue tracker :)

### What testing has been done on this change?

`npm test`.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/filmaj/cordova-lib spec-consolidation

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-lib/pull/568.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #568


commit bc4218e3d09682b0bdcd5a2a119e29c0561a03ea
Author: filmaj 
Date:   2017-06-22T20:29:57Z

Reorganized unit test directory a little bit, consolidated into a single 
spec/ dir. Put jasmine config and helper modules in top-level spec dir. Changed 
package.json npm run scripts to reflect purposes of tasks. Updated readme to 
reflect package.json npm run script changes.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org