Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring for better testability #31

Merged
merged 14 commits into from Aug 30, 2016
Merged

Refactoring for better testability #31

merged 14 commits into from Aug 30, 2016

Conversation

mstade
Copy link
Member

@mstade mstade commented Aug 23, 2016

Description

With #30 we gained some initial scaffolding for tests, and with that we'll start increasing the test coverage of this project. To that end, this PR aims to refactor ez-build to make it more testable, and ideally easier to implement new functionality as well. Additionally, integration with code coverage tooling is another explicit goal of this work.

Motivation and Context

While #30 gave us a good start, it didn't include code coverage, and also highlighted that some parts (the main routine, for instance) was muddied by mixing too many concerns.

How Was This Tested?

Additional tests are included, to increase coverage, and instrumented building is enabled for the test script as well, to ensure we can get accurate code coverage reports on every build.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change follows the style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the contribution guidelines
  • I have added tests to cover my changes
  • All new and existing tests passed

This cleans up the main process, and makes options parsing easier
to test.
Also added tests for pkg parsing, opts parsing tests to follow
For instrumentation we use babel-plugin-istanbul, which instruments
code for subsequent collection and reporting. For these latter
two tasks, we use nyc. We also use codecov.io integration for
checking status on GitHub.
@codecov-io
Copy link

codecov-io commented Aug 23, 2016

Current coverage is 78.63% (diff: 91.17%)

No coverage report found for master at e73ea49.

Powered by Codecov. Last update e73ea49...c29240b

@Poetro
Copy link
Member

Poetro commented Aug 23, 2016

👍

If not, the instrumentation will occur on already compiled code.
This means that when we run code under test, it's JIT compiled as
opposed to AOT compiled – this may cause problems if the config
where to fall out of sync between JIT and AOT versions.
If we clean up, things like coverage reports will be removed and
we won't get nifty graphs. Lack of nifty graphs makes me sad in
the face.
This reverts the change to make unit tests load JIT compiled code
and instead loads AOT compiled code instead. This makes sense, as
we want to test the output, not precompiled source, and make sure
we never get out of sync in terms of configuration.

As well, this makes code coverage a distinct step from normal
testing, so as not to incur performance penalties in an iterative
development process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants