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

postgresql integration tests #73

Closed
wants to merge 1 commit into from

Conversation

lwsanty
Copy link
Contributor

@lwsanty lwsanty commented Sep 16, 2019

Signed-off-by: lwsanty lwsanty@gmail.com

@lwsanty lwsanty force-pushed the pq_integration_tests branch 3 times, most recently from 29439b6 to 229e0f1 Compare September 16, 2019 16:43
@lwsanty lwsanty changed the title postgresql integration tests [WIP] postgresql integration tests Sep 17, 2019
@lwsanty lwsanty changed the title [WIP] postgresql integration tests postgresql integration tests Sep 17, 2019
@lwsanty lwsanty requested a review from a team September 17, 2019 12:04
@lwsanty lwsanty self-assigned this Sep 17, 2019
@lwsanty lwsanty requested review from mcarmonaa and jfontan and removed request for a team September 17, 2019 16:16
Copy link
Contributor

@jfontan jfontan left a comment

Choose a reason for hiding this comment

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

There's a regression in the coverage, can you take a look? Could it be related with GITHUB_TOKEN not being available? Travis hides secret tokens to PRs done from other repositories. You can make use of the token if you push the branch to the repository and you create the PR from it.

It would be nice that the integration test uses a temporary database for each test. This makes local testing easier as you don't have to delete the database before each test. We are already doing in data-retrieval and I believe the code could be copied almost verbatim:

https://github.com/src-d/data-retrieval/blob/master/test/database.go

@lwsanty
Copy link
Contributor Author

lwsanty commented Sep 17, 2019

@jfontan i think most probably the regression error is related to token, should I create new PR after I push to the branch of this repo?

@jfontan
Copy link
Contributor

jfontan commented Sep 18, 2019

@lwsanty yes, upload to a branch in this repo and create a new PR.

Signed-off-by: lwsanty <lwsanty@gmail.com>
@lwsanty
Copy link
Contributor Author

lwsanty commented Sep 18, 2019

closing this one because new PR was created #77

@lwsanty lwsanty closed this Sep 18, 2019
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.

2 participants