search

kblincoe / VisualGit_SE701_2019_1

1 stars 0 forks source link

Fixes #71 - Adds automated test runner #150

Open oscarcs opened 5 years ago

oscarcs commented 5 years ago

Related Issue: Closes #71

Description: Adds Karma as a test runner. Adds Jasmine as test description framework. Adds karma-typescript, so that we can write tests in Typescript. Adds karma-electron to run the tests in a headless electron instance. Run using npm test.

Stuff that possibly needs to be done before this can be merged:

oscarcs commented 5 years ago

Note that this PR allows testing of Typescript components, but until we have no Typescript compilation errors, we can't test any of the app code (because this will cause the tests to fail). This PR does not cover testing of Angular components. I don't think any of that is necessarily blocking for this PR, but we need to have some discussion around it.

KelseyRM commented 5 years ago

For anyone else having a look through this I found this http://karma-runner.github.io/3.0/config/configuration-file.html very useful to understand what the config options are

KelseyRM commented 5 years ago

I have been playing around with getting basic testing working using this setup - an idea from @bcox280 was to get the --auto-watch going so keep Karma running despite build errors as this may have also bypassed the issue where compilation errors were stopping tests from passing. However this doesn't seem to work - I'm not sure if it's because we're using Electron or how the project uses Angular or something along those lines, but no config variation that I have been able to find gets around this. Moving on to writing tests for services, I found that the services need to be registered in the app component module in order for the karma/electron instance to be able to find them. This is an issue as the codebase currently has no modules at all, so a decent rewrite would be required to utilise them. Basically, until we can register the services in a module the tests can't find them even if there aren't any errors in the .spec file itself. I'm not sure what else we can test, since from what I've found (although I may be wrong) even the misc files seem to need modules to be able to access them and their methods from the testing files.

oscarcs commented 5 years ago

Yeah so I also had tried out auto-watch but found that it didn't help us, so I thought that probably running the tests as a discrete pass would be preferable from a UX perspective.

Do we want to start creating issues (or referencing here existing ones) that are blocking for us to merge this PR? I find it unlikely that we're going to get to most or even any of them before the deadline, but from a process point of view it might be a good idea.

oscarcs commented 5 years ago

End-note about this PR: we will be leaving it open as it's fine to be merged, but is not worth merging until a number of quite complex tasks are completed, including moving to a new version of Angular and performing refactoring on the codebase. (We will not have time for these tasks during the project timeframe.)