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

Moved apps to /services/ & moved individual tests #9187

Merged
merged 8 commits into from
Oct 30, 2017

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Oct 29, 2017

refs #9178

This moves the code that is related to managing the app lifecycle & sandbox into the /services/ folder, so that server/apps only contains internal apps.

It also moves the individual app tests to the /tests/ folder. This is because those tests were assumed to be unit tests, but not all of them were. The subscribers tests were functional route tests. By doing this, we reveal the true level of unit testing of the apps code:

Before:

After:

This makes it clear that the app lifecycle needs deeper unit testing & probably should feed into the discussion about moving one set of tests (either integration or functional).

Note: 2 timezone-related tests are failing on my local machine this morning, which I guess is DST-change related.

refs TryGhost#9178

- Apps is a service, that allows for the App lifecycle
- /server/apps = contains internal apps
- /server/services/apps = contains code for managing/handling app life cycle, providing the proxy, etc
- Problem: Not all the tests in apps were unit tests, yet they were treated like they were in Gruntfile.js
- Unit tests now live in /test/unit/apps
- Route tests now live in /test/functional/routes/apps
- Gruntfile.js has been updated to match
@ErisDS ErisDS merged commit 882a236 into TryGhost:master Oct 30, 2017
@ErisDS ErisDS deleted the apps-as-service branch October 30, 2017 12:31
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.

1 participant