Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Test plugin improvements #46

Merged
merged 2 commits into from
Feb 8, 2018
Merged

Test plugin improvements #46

merged 2 commits into from
Feb 8, 2018

Conversation

hwillson
Copy link
Contributor

While prepping an answer for issue #8, I took a quick look at the current cordova-plugin-meteor-webapp testing approach. Unless I'm missing something (which is entirely possible!) it looks like the current tests haven't been run in quite some time, as the iOS tests specifically can't be run due to Swift compilation errors.

To help with this, this PR addresses the following cordova-plugin-meteor-webapp / cordova-plugin-meteor-webapp-tests issues:

  • Fixes Swift compilation errors preventing the iOS tests from running.
  • Adds a package.json so the test plugin can be added as a separate plugin with modern versions of Cordova.
  • Adjusts the tests after_plugin_install call to reference the iosAddBridgingHeader.js helper file as if it was part of the cordova-plugin-meteor-webapp-tests plugin. Otherwise the iosAddBridgingHeader.js file can't be found when cordova-plugin-meteor-webapp-tests is added via cordova plugin add.

This PR also includes a new DEVELOPMENT.md file that contains detailed instructions outlining an approach for running the cordova-plugin-meteor-webapp-tests iOS tests (using local checkouts to make debugging easier).

The above being said, this PR does NOT address the failing iOS tests (38 out of 69 iOS tests are failing). It just adjusts the testing infrastructure so we can at least see the failing tests. I'll look into addressing the failing tests in a separate PR.

Fixes #8.

Thanks!

This commit introduces the following changes to the
`cordova-plugin-meteor-webapp-tests` (`/tests`) plugin:

- Fix Swift compilation errors preventing the iOS tests from
  running.
- Add a `package.json` so the test plugin can be added as a
  separate plugin with modern versions of Cordova.
- Adjust the tests `after_plugin_install` call to reference the
  `iosAddBridgingHeader.js` helper file as if it was part of the
  `cordova-plugin-meteor-webapp-tests` plugin. Otherwise the
  `iosAddBridgingHeader.js` file can't be found when
  `cordova-plugin-meteor-webapp-tests` is added via
  `cordova plugin add`.
@abernix
Copy link
Contributor

abernix commented Jan 24, 2018

Thanks very much for looking at this. I mostly support the type of Swift changes you've made here, even if I don't understand when exactly they became required.

I'm curious if the test suite passes more successfully if the GCDWebServer submodule is (temporarily) moved to meteor/GCDWebServer@8a6a139 (commit hash), which was what I believe it was at before it became a submodule (rather than a copy) in #45. I'm hoping those pass which might help us further validate some of the GCDWebServer changes which probably need to be made?

@hwillson
Copy link
Contributor Author

Great question @abernix - I'll test things out and post back shortly.

@abernix abernix self-assigned this Jan 31, 2018
When the `cordova-plugin-meteor-webapp-tests` plugin is
installed, it ends up in an apps `plugins` directory, at
the same level as the `cordova-plugin-meteor-webapp`
plugin. This commit updates the path the tests use to
find the shared `scripts` directory (which is in the
`cordova-plugin-meteor-webapp` plugin) when installed.
@abernix abernix merged commit 3e636e9 into meteor:master Feb 8, 2018
@abernix
Copy link
Contributor

abernix commented Feb 8, 2018

I think getting these tests running again is likely highly beneficial, so I've merged this. I should have a follow-up PR soon which will help run these tests automatically, rather than needing to follow all the steps in this new DEVELOPMENT.md, but good to have those documented regardless.

I did encounter a problem with this PR when I first tried it but it was fixed when @hwillson added the second commit (a7d4868). That commit is also proving to be important when trying to run the tests when the repository was in an older state, so I'm maintaining it upon merging this, and not squashing it out.

Thanks again for working on this!

abernix added a commit that referenced this pull request Feb 8, 2018
By using TravisCI's Mac testing environment, which comes fully-equipped
with Xcode and iOS simulators, we can run the test suite which @hwillson
revived in #46, automatically.

While many of the tests are failing, there are some passing, which seems
to prove that the testing framework is functioning properly, though
doesn't provide much assurance that we'll be able to fix the tests (though
it's likely, and I'm hopeful, that we can!).

Despite hesitance to do so, I've specifically pinned the iOS simulator to
use iOS 11.2 since the `cordova-paramedic` npm tries to obtain the logs from
the simulator by looking through the output of `instruments -s devices`, though
the `cordova` npm is only aware of minor versions in iOS releases, not patch
versions (e.g 11.0.1, which it classifies as 11.0).  Since Travis is
automatically updating to patch versions of iOS, presumably in an effort
to solve the security problems or deficiencies the initial releases had,
this seems necessary for the time being.

Anyone should bump the simulator OS version as necessary/beneficial.  We could
potentially fix this in our own fix to `cordova-paramedic`, which I've
already had to fork for unrelated reasons, but I haven't taken the time
to investigate whether this is reasonable.

This also adds `ios-deploy` and `ios-sim` as `devDependencies` for testing, and
removes them from their previous installation location in `.travis.yml`,
which makes it possible to run `npm test` locally.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants