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 <maj....@gmail.com>
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
