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

EHPR-186 | Integration tests #142

Merged
merged 14 commits into from
Jan 12, 2022
Merged

EHPR-186 | Integration tests #142

merged 14 commits into from
Jan 12, 2022

Conversation

arranfw
Copy link
Contributor

@arranfw arranfw commented Jan 10, 2022

  • make api-integration-test to run integration tests with a transient postgres container
  • Integration tests use their own DB config + env vars
  • Test docker-compose for test db + make commands
  • Fix up integration test setup
  • Extract validationpipe config for use in integration testing
  • sendMailable returns void if chesHost doesn't exist
  • Add basic tests for submissions

@arranfw arranfw force-pushed the EHPR-186-Integration-Tests branch 6 times, most recently from 73e3d00 to d33f227 Compare January 11, 2022 00:09
Copy link
Contributor

@fwpushan fwpushan left a comment

Choose a reason for hiding this comment

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

Code-wise good, but please check my comments and make sure integration tests should pass in github action

Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@arranfw arranfw force-pushed the EHPR-186-Integration-Tests branch 7 times, most recently from da32d60 to 75c350c Compare January 11, 2022 16:58
@arranfw arranfw marked this pull request as ready for review January 11, 2022 17:24
@echo "++\n*****"

start-test-db:
NODE_ENV=test docker-compose -f docker-compose.test.yaml up --build -d
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels a bit weird to have a separate db docker container for testing.
We should be able to reuse the same connection for the main app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make it a completely separate database so it could be configured slightly differently and so it wouldn't interfere with the main app if it's already running, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Normally we wiped-out (tear down) db for correctness of test and isolation of test execution. In that case using separate database is good choice.

@sonarcloud
Copy link

sonarcloud bot commented Jan 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@fwpushan fwpushan left a comment

Choose a reason for hiding this comment

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

LGTM. Good job

@arranfw arranfw merged commit 464629f into main Jan 12, 2022
@arranfw arranfw deleted the EHPR-186-Integration-Tests branch January 12, 2022 23:11
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.

3 participants