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

Continuous Integration #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions text/0002-continous-integration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
- Start Date: 2020-03-07
- RFC PR: https://github.com/ReCreateJS/rfcs/pull/2
- Relevant Project: All CreateJs Projects

# Continuous Integration

## Summary

Continuously test the CreateJS source on PRs and releases.

## Motivation

The practice of continuous integration gives high confidence that contributed changes do not break the project. The project can be automated to improve in quality using appropriate tooling.

## Detailed design

Set up TravisCI script that runs all automated forms of testing.

This will include:

1. Cross-browser Javascript test suite
2. Test coverage checking
3. Static code linting tooling with ESLint
Copy link
Contributor Author

@jedateach jedateach Mar 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't state which ESLint rules to apply, or how they would be applied.

To make this work easier to digest+decide, I'll this out to it's own RFC....and probably the documentation coverage checking it also.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint rules might be tricky so I think it's a good idea to make a separate RFC. I ran the combined build against the rules I use and a lot of things came up, such as extensive use of '==' instead of '==='.

I think the temptation would be to just adjust the rules so that the current build passes.

4. Documentation coverage checking

Every PR will build and test the project on TravisCI, providing an indication on the PR as to whether the change has broken anything.

GitHub will be configured to prevent PRs being merge unless the travis build completes.

### Run existing tests on multiple devices

There already exists a test suite, that will be run on Chrome headless in Travis.

Assuming visual regression tests only work on one browser, the remaining tests can be also run on:

- Firefox Headless
- Node
- SauceLabs (lowest supported browser)

### Test coverage

Using Istanbul, source code will be instrumented and run to record which lines have been run by tests.

The coverage report will be sent to Code Climate for analysis.

If test coverage drops, or a change isn't covered by tests, then code climate will reject a PR.

### Highlighting CI/CD status

Add a badge to the main README.md file, showing the master build status. This should always be green, otherwise additional PRs should not be merged until it is fixed.

## How we teach this

Update the GitHub CONTRIBUTING.md and ISSUE_TEMPLATE.md files to explain how continuous integration works.

## Drawbacks

If users are unfamiliar with writing tests, then they may be less inclined to contribute.
That is not to stop a PR having tests written by someone else.

## Alternatives

Manual testing changes is possible, but requires people's time, which is better spent in other ways.

Other similar frameworks like PIXI.js are using these practices.

## Unresolved questions

- Use Code Climate, codecov.io, coveralls or other for coverage analysis?
- Use Saucelabs or something else to do further cross-device testing?
- Are there any other tools that can help?