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 <[email protected]>
Date: 2017-06-19T22:18:17Z
the existing plugin specs are all e2e tests... moving them to relevant
directory.
commit c773cd2e9a869846b472464d347738404089720a
Author: filmaj <[email protected]>
Date: 2017-06-19T22:25:38Z
plugin e2e test changes to account for dir move
commit 8d3d6108d86d5c7258c3613ba9d4f16ec3cf439c
Author: filmaj <[email protected]>
Date: 2017-06-19T23:45:31Z
first pass at plugin command refactor
commit 3d17ab4eb4ac068bc0e31f7d1a6fecb6a77b711a
Author: filmaj <[email protected]>
Date: 2017-06-20T03:54:26Z
refactor of code should be done.
commit 4218ea522845030c24f64ba6aa14340eb34c36be
Author: filmaj <[email protected]>
Date: 2017-06-20T04:02:29Z
deprecate anything related to plugin_parser class.
commit 10b24897a21ee319cffda976667e755497b63328
Author: filmaj <[email protected]>
Date: 2017-06-20T04:10:13Z
eslint fixes for plugin specs
commit a75986076daf2b0fde84f2d1e9dbb08f55858869
Author: filmaj <[email protected]>
Date: 2017-06-20T04:23:18Z
plugin_fetch.spec.js are integration tests - moving to integration-tests dir
commit 9a0217522473a6fb1a562afb6863112dd3fa7cc2
Author: filmaj <[email protected]>
Date: 2017-06-20T04:29:22Z
hooks runner tests are also e2e - moving to integration-tests dir
commit 0677b0463034467c422e044c53b31604dae84b07
Author: filmaj <[email protected]>
Date: 2017-06-20T04:36:29Z
moved plugman fetch specs to integration tests dir
commit d1b8580a671f6ee818a8d12a30611ac99c3f055c
Author: filmaj <[email protected]>
Date: 2017-06-20T04:50:17Z
last of i/o heavy tests into integration-tests dir
commit 9cc1e14a4211b11888b486762f9265a703bad135
Author: filmaj <[email protected]>
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 <[email protected]>
Date: 2017-06-20T19:20:20Z
start of unit tests for cordova.plugin commands
commit 803c3c88c32b39862ee57f4a2b76cdef1b81de0b
Author: filmaj <[email protected]>
Date: 2017-06-20T19:20:54Z
fixes to integration test refrences so they run
commit 29a47cb7d28b974a131cb8f0ac048724de656b36
Author: filmaj <[email protected]>
Date: 2017-06-20T19:21:09Z
todos for cordova.plugin commands.
commit ed888914d85c35b327c606e523953d3e6e6d3ff3
Author: filmaj <[email protected]>
Date: 2017-06-20T19:42:41Z
bye bye save integration specs.
commit c848f7648a663e9b70fa02627f366828da145ca8
Author: filmaj <[email protected]>
Date: 2017-06-20T19:43:22Z
nest certain plugin-remove-save tests into a describe to better reflect
structure
commit 81b7b228742b1380d0d95956b7b7d15138a75b19
Author: filmaj <[email protected]>
Date: 2017-06-20T19:49:46Z
:snail: bad refs to prepare
commit 5d86ebb9b1ee3c4c4801551b454a74f9dbf8f2f0
Author: filmaj <[email protected]>
Date: 2017-06-20T19:56:39Z
todo to move plugin fetch integration tests into plugin/add/fetchVersion
unit tests.
commit 5e5109f5e20147d48a1f670c656324e7e7353a91
Author: filmaj <[email protected]>
Date: 2017-06-20T23:36:47Z
pending tests for plugin specs
commit 5a2e5e96bb38fc1edaf1f7c15758861d4b99c375
Author: filmaj <[email protected]>
Date: 2017-06-21T14:24:45Z
small refactor tweaks, mostly TODOs being dropped
commit 2dc98da5883557f19cbfb08486ba4e02f97f376c
Author: filmaj <[email protected]>
Date: 2017-06-21T14:32:21Z
remove old platform-add tests. fixes for jshint.
commit 63c762cb7d8cfbbdacbbfa32cc4321c041c958cd
Author: filmaj <[email protected]>
Date: 2017-06-21T14:39:51Z
plugin util specs implemented
commit bb524e51e98be77ebdf6576b4385fdf46d629ca7
Author: filmaj <[email protected]>
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
