cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From filmaj <...@git.apache.org>
Subject [GitHub] cordova-lib pull request #568: Reorganized unit test directory structure + u...
Date Thu, 22 Jun 2017 20:44:58 GMT
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.fil@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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


Mime
View raw message