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

Add unit/e2e tests for mongo-typeorm sample #3173

Closed
wants to merge 10 commits into from

Conversation

kiwikern
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Other... Please describe: Docs / Samples

What is the current behavior?

Issue Number: #1539

What is the new behavior?

Added unit + e2e tests for mongo+typeorm sample.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@kiwikern kiwikern changed the title Mongo typeorm sample tests Add unit/e2e tests for mongo-typeorm sample Oct 12, 2019
@coveralls
Copy link

coveralls commented Oct 12, 2019

Pull Request Test Coverage Report for Build c6f66b77-4cf5-4dc2-a4bc-d25738b16a38

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.818%

Totals Coverage Status
Change from base Build 736a64e8-08de-40ee-9990-faac31009973: 0.0%
Covered Lines: 4922
Relevant Lines: 5191

💛 - Coveralls

Copy link
Member

@BrunnerLivio BrunnerLivio left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your PR @kiwikern !
Could you rebase with master - we run sample e2e & unit tests now in CI :)

@kamilmysliwiec how shall we run the e2e tests with the database in CI? Either we use

  • SQLite so we would not need a docker-compose file?
  • We run docker-compose up && jest --config ./test/jest-e2e.json as npm run test:e2e script?
  • Update the test:e2e:samples task to make it work with docker-compose out of the box?

sample/13-mongo-typeorm/test/photos.e2e-spec.ts Outdated Show resolved Hide resolved
sample/13-mongo-typeorm/test/photos.e2e-spec.ts Outdated Show resolved Hide resolved
@kamilmysliwiec
Copy link
Member

Update the test:e2e:samples task to make it work with docker-compose out of the box?

How would that look like?

@BrunnerLivio
Copy link
Member

@kamilmysliwiec Check whether the sample has a docker-compose.yml file in the root directory and run docker-compose up -d if so.

… mongo-typeorm-sample-tests

# Conflicts:
#	sample/13-mongo-typeorm/package-lock.json
#	sample/13-mongo-typeorm/package.json
#	sample/13-mongo-typeorm/test/photos.e2e-spec.ts
@kiwikern kiwikern force-pushed the mongo-typeorm-sample-tests branch from 6189a8c to e3696cd Compare November 3, 2019 23:13
@kiwikern
Copy link
Contributor Author

kiwikern commented Nov 3, 2019

Thanks for your feedback @BrunnerLivio. 🙂 I addressed your comments and rebased to master. As expected, the e2e test is now failing since the database is not started.

# Conflicts:
#	sample/13-mongo-typeorm/package-lock.json
#	sample/13-mongo-typeorm/package.json
@kamilmysliwiec
Copy link
Member

Since the only thing the controller does is delegating work to the injected service instance (which we mock in this test), it doesn't really bring any value (does not protect us from anything).

@kiwikern
Copy link
Contributor Author

@kamilmysliwiec I absolutely agree that these tests do not ensure the example code does not break. However, I believe the goal of the tests is to rather serve as examples on how to set up unit/e2e tests for the mongo-sample given. Especially, how you can mock a repository can be of interest for new users.

From the linked issue #1539:

Give new users like me simple and practical examples for implementing tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants