Spike: Front End Test Improvement
Aperta is large project that has a lot of moving parts and complex logic. As responsible developers, we try our best in ensuring that the features we built work as advertised by following a TDD pattern as we go. In general, the coverage within our Rails app, while not exactly perfect, it's at least quite extensive and reasonable (a previous report by Jim suggested that our code coverage there is roughly 90%+). Our front end coverage (Herein referred to as Ember QUnit tests, or QUnit), however, could use various improvements.
During the earliest part of the project, there wasn't quite as many QUnit tests written. A part of this could of been related to the Tahi project switch to Ember, the difficulty of writing Ember QUnit tests at the time, the change in Web Development culture doing front-end testing at all (arguably, it only took off in the late 2000s), or other reasons. The Aperta team has been plugging up said holes as the project went on to the point where, after a quick good look through, actually is a lot more comprehensive than I thought.
Multiple devs, including myself, have looked into setting up a frontend code coverage tool in order to discover where the gaps are, but have been having difficulty getting it to run without issues . I have only recently got it (Ember CLI Code Coverage, to be specific) to complete a run (although the tests themselves were still running into issues) and was able to get it return a code coverage number of roughly 60%. If you're curious of the specific results, you can copy the directory the directory on the shared drive at (sfnas02.plos.org/PLoSShares/Aperta/APERTA-11433/coverage) and view it in your browser of choice.
Jack has been able to get Ember CLI Code Coverage cleanly to run by downgrading - the last report was somewhere around 80%, which is somewhat surprising to me, but that could be because that was ran pre-engine migration related to APERTA-12039.
-
If an acceptance test
-
Set up mockjax handlers (in certain cases, we use a Sinon fakeServer). We tend to do a lot of Mockjax handler setup across the various acceptance tests, so it would be great if they could be unified. In a previous project, this was accomplished using http://www.ember-cli-mirage.com/ since it uses a mock server (it uses Pretender under the hood). This has the benefit of ensuring that our handlers are set up correctly (a common mistake is a mismatched Mockjax handler that never gets called) since we only have to check once, as well as getting a lot of the mock setup out of the tests.
-
Add in Ember Data models (one constant part being setting up permissions). A lot of this setup comes from methods located in Factory.js (a complicated looking file as a result of a old 'decaffeinated' CoffeeScript file). We could offset this somewhat by leveraging newer features within FactoryGuy to do a lot of this complicated belongsTo/HasMany logic (which is a decent chunk of the complexity) and move them into the individual factory files themselves (example). We can also use Ember CLI Mirage to improve this since it has nice features like (finally) being able to support polymorphic setups, schema auto detection, and so on.
-
-
If a component integration test
-
Set up Mockjax handlers in a lot of cases. Personally, I believe that a lot of these Mockjax handlers could be using something along the line of Sinon spys, stubs and mocks instead as I believe they're less complicated to setup most of the time. For example, instead of using a mockjax handler to bridge between what we're ultimately asserting against, we can just return a mock response to keep it moving or a spy to just check that a certain method has been called. We would likely get some performance boosts if we can limit overcomplicated setups too.
-
Create mock objects that we generate using FactoryGuy factories. We break standard convention (well, it's more FactoryGuy doing it, but we openly telling FactoryGuy to do it
{.emoticon
.emoticon-smile}) in that we initialize a component integration
test with Ember Data loaded. I'm kinda on the fence about this
approach: It does make create mock objects easy in these tests,
but I think it encourages to overcomplicate what we pass into
our components and should be a warning sign about how we design
them.
-
-
The unit tests seem fine to me (by their nature, they tend to be simple to setup - although they tend to be the tests that have holes in coverage)
I had success in the past migrating a FactoryGuy dominate QUnit acceptance test suite into a more Mirage dominate one (see Lemur-Collections in the PLOS repo if you're curious). I've tried experimenting with integrating Mirage since it handles issues that we normally run into during our Acceptance test runs (we optionally could use some of FactoryGuy's newer features - we had a big upgrade recently with the Ember Upgrade, but I don't think many are aware of the changes).
Normally, how I expected it to happen would be like I done it before.
- We install Mirage and migrate our existing Factory Guy factories to be Mirage factories where applicable
- Every time a dev touches an acceptance file, we migrate old Mockjax handler behavior to Mirage's server.js. This is easier than it sounds since Mockjax and Mirage plays fairly nice with each other in regards to its FakeXHR magic. When a test runs and a request is made, it will check to see if there's a Mockjax handler already set and use that (so, for existing tests, it will work as they did before). If there's no Mockjax handler available, it will check to see if Mirage has a mock endpoint available. If neither are found, the test will throw an error, provides a useful stack trace (at least from what I remember), and we can then address it.
- Eventually, there will be enough scaffolding for the common cases to starting consistently removing the previous handlers without having to add to server.js and we can handle less common cases as we go.
There has been some problems in the past in that Ember CLI Mirage didn't support polymorphic relationship (which was unfortunate since Lemur had a lot of such cases... and so does Aperta), but that has been addressed with Mirage's 3.0 release, so I was excited to try this out with one of our smaller Acceptance tests.
Things didn't quite work out well after just installing the library. Various errors regarding Broccoli build issues and a bizarre second Pretender server existing somewhere was taking too much time for what we had originally prioritize. I still think it's worth investigating the feasibility Mirage 3.0 for Aperta.
TL:DR
- We should still consider using Mirage - I don't think FactoryGuy and it are mutually exclusive, but we should be aware that Mirage currently doesn't play nice with Aperta and it isn't as quick to setup as originally though.
- We should be using Sinon considerably more often in our Component Integration tests (and also just ensuring they're less complicated in nature in general, but that's easier said than done).
- The code coverage percentage is higher than I thought, but there's still spots that could use some love. We should use https://github.com/kategengler/ember-cli-code-coverage after working out the kinks and spend a bit of time knocking out the more critical holes (See APERTA-12320).