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 #566: Plugin specs
Date Wed, 21 Jun 2017 15:35:06 GMT
GitHub user filmaj opened a pull request:

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

    Plugin specs

    *WARNING*: Work in progress! Please review and leave comments.
    
    ### What does this PR do?
    
    A lot! The main purposes of this PR are:
    
    1. Address removing lazy loading of submodules that was removed in a previous PR, and
how that affected functionality across certain subcommands like the plugin subcommand.
    2. Refactor the plugin command.
    3. Do what is necessary to get CI working again for this repo.
    4. Comb over the tests for the plugin command, and split the tests into integration tests
(ones that rely on fixtures and do heavy file and/or network I/O) and unit tests (fast, mocked,
logic-focussed and scoped to individual modules-under-test).
    5. Get the unit tests to run in a matter of seconds, not minutes (runs in 2 seconds on
my machine now - check by running `npm run jasmine`)
    6. Remove any integration tests that exercise code that is already tested via unit tests
(ideally making CI and test runs faster - runs in a couple minutes now on my machine).
    
    In detail:
    
    - Refactored the `cordova.plugin` command and split it up into submodules along the lines
of its subcommands, i.e. `src/cordova/plugin/index` is the main entry point, but now there
also exist `add`, `remove`, `list`, etc. submodules. Makes the submodules smaller and hopefully
more easily understandable, as well as makes them more testable.
    - Added the initial first steps towards unit test coverage for the plugin command. Wrote
_a lot_ of pending unit tests for what I believe are high-level goals of each submodule /
helper function.
    - Moved a whole bunch of tests that are integration tests into the integration-tests directory.
    - Removed a whole bunch of integration tests that exercised code we already had code coverage
for, either in other integration tests, or in unit tests.
    
    *Worth Noting*:
    - There are still plenty of unit tests to fill out and write for the plugin command -
this PR is a work in progress and I figured it would be better to issue the PR and get eyes
on it early rather than wait til the tests were written. I believe getting this PR reviewed
and proofed by the other committers, to ensure I am on the right track, was more important
than writing out all the pending tests. That effort can be done in parallel. It is worth discussing
whether we want to have the pending tests written up first before we do a next release (I
am of the opinion that yes, we should ensure we write up any pending unit tests before doing
the next release).
    - The tests in plugin.add for plugin version constraint satisfaction helper methods (`getFetchVersion`,
`determinePluginVersionToFetch`, `getFailedRequirements`) _do not have pending unit tests
written up for them yet_, as, honestly, that code confused the hell out of me. I believe @stevengill
was the last one to work on those tests and that code, so I would like to work more closely
with Steve to get a better understanding of what and how these methods do what they do. Luckily,
we still have test coverage for these methods (under `integration-tests/plugin_fetch.spec.js`),
however, I would like to rewrite or migrate these tests so that they do not rely on fixtures
and file I/O.
    - These are big changes, so any pending PRs will likely need to be rebased once this lands.
    
    ### What testing has been done on this change?
    
    ran unit tests, e2e tests and jshint.
    
    FYI (or, even better, please review!) @stevengill, @audreyso, @purplecabbage, @shazron,
@imhotep, @kerrishotts, @dpogue, @matrosov-nikita, @goya 

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

    $ git pull https://github.com/filmaj/cordova-lib plugin-specs

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

    https://github.com/apache/cordova-lib/pull/566.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 #566
    
----
commit e33630d3b2e0eb3e948f7bda1435b511f8e93334
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-19T22:18:17Z

    the existing plugin specs are all e2e tests... moving them to relevant directory.

commit c773cd2e9a869846b472464d347738404089720a
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-19T22:25:38Z

    plugin e2e test changes to account for dir move

commit 8d3d6108d86d5c7258c3613ba9d4f16ec3cf439c
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-19T23:45:31Z

    first pass at plugin command refactor

commit 3d17ab4eb4ac068bc0e31f7d1a6fecb6a77b711a
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T03:54:26Z

    refactor of code should be done.

commit 4218ea522845030c24f64ba6aa14340eb34c36be
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:02:29Z

    deprecate anything related to plugin_parser class.

commit 10b24897a21ee319cffda976667e755497b63328
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:10:13Z

    eslint fixes for plugin specs

commit a75986076daf2b0fde84f2d1e9dbb08f55858869
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:23:18Z

    plugin_fetch.spec.js are integration tests - moving to integration-tests dir

commit 9a0217522473a6fb1a562afb6863112dd3fa7cc2
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:29:22Z

    hooks runner tests are also e2e - moving to integration-tests dir

commit 0677b0463034467c422e044c53b31604dae84b07
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:36:29Z

    moved plugman fetch specs to integration tests dir

commit d1b8580a671f6ee818a8d12a30611ac99c3f055c
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:50:17Z

    last of i/o heavy tests into integration-tests dir

commit 9cc1e14a4211b11888b486762f9265a703bad135
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T04:56:02Z

    removing unnecessary else from plugin remove. moved plugin-package-parse to plugin subdir
and renamed to plugin_spec_parser (to reflect name of module under test).

commit f496408e6f9c56a1b8f959edc075e07df0ba9812
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:20:20Z

    start of unit tests for cordova.plugin commands

commit 803c3c88c32b39862ee57f4a2b76cdef1b81de0b
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:20:54Z

    fixes to integration test refrences so they run

commit 29a47cb7d28b974a131cb8f0ac048724de656b36
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:21:09Z

    todos for cordova.plugin commands.

commit ed888914d85c35b327c606e523953d3e6e6d3ff3
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:42:41Z

    bye bye save integration specs.

commit c848f7648a663e9b70fa02627f366828da145ca8
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:43:22Z

    nest certain plugin-remove-save tests into a describe to better reflect structure

commit 81b7b228742b1380d0d95956b7b7d15138a75b19
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:49:46Z

    :snail: bad refs to prepare

commit 5d86ebb9b1ee3c4c4801551b454a74f9dbf8f2f0
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T19:56:39Z

    todo to move plugin fetch integration tests into plugin/add/fetchVersion unit tests.

commit 5e5109f5e20147d48a1f670c656324e7e7353a91
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-20T23:36:47Z

    pending tests for plugin specs

commit 5a2e5e96bb38fc1edaf1f7c15758861d4b99c375
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-21T14:24:45Z

    small refactor tweaks, mostly TODOs being dropped

commit 2dc98da5883557f19cbfb08486ba4e02f97f376c
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-21T14:32:21Z

    remove old platform-add tests. fixes for jshint.

commit 63c762cb7d8cfbbdacbbfa32cc4321c041c958cd
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-21T14:39:51Z

    plugin util specs implemented

commit bb524e51e98be77ebdf6576b4385fdf46d629ca7
Author: filmaj <maj.fil@gmail.com>
Date:   2017-06-21T15:17:18Z

    first unit test in each plugin submodule, jshint fixes

----


---
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