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.

Sign up for GitHub

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

Jump to bottom

fix(chore): Orchestrate test binary and docker compose files to report coverage for integration tests #8494

Merged
merged 51 commits into from
Dec 9, 2022
Merged

fix(chore): Orchestrate test binary and docker compose files to report coverage for integration tests #8494

merged 51 commits into from
Dec 9, 2022

Conversation

all-seeing-code
Copy link
Contributor

Problem

Coverall is unable to capture code coverage without binary tooling

Solution

PR builds on @kevinmingtarja 's work in #8450. It adds environment variables to docker compose files so that we can run containers with and without coverage mode turned on.

@github-actions github-actions bot added area/graphql Issues related to GraphQL support on Dgraph. area/integrations Related to integrations with other projects. area/testing Testing related issues labels Dec 7, 2022
@coveralls
Copy link

coveralls commented Dec 7, 2022 •
edited
Loading

Coverage Status

Coverage increased (+26.7%) to 63.976% when pulling fa342b2 on anurag/system-test-coverage into 7326d72 on main.

all-seeing-code
all-seeing-code commented Dec 7, 2022
View reviewed changes
dgraph/Makefile Outdated Show resolved Hide resolved
all-seeing-code
all-seeing-code commented Dec 7, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
all-seeing-code
all-seeing-code commented Dec 7, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@kevinmingtarja kevinmingtarja mentioned this pull request Dec 7, 2022
Closed
kevinmingtarja and others added 20 commits December 8, 2022 01:38
@kevinmingtarja @all-seeing-code
Create main_test.go as entrypoint for the test binary
069456c
@kevinmingtarja @all-seeing-code
Add makefile cmd to generate test binary and image
e2afc25
@kevinmingtarja @all-seeing-code
Use new makefile cmd in ci-dgraph-tests
0fb7b3a
@kevinmingtarja @all-seeing-code
Refactor t.go to support code coverage for system tests
15a0abd
@kevinmingtarja @all-seeing-code
Refactor docker-compose of poorman_auth test
65663b0
@kevinmingtarja @all-seeing-code
Change test binary name
9691a5e
@kevinmingtarja @all-seeing-code
Add step to remove tmp coverage files
88e270d
@kevinmingtarja @all-seeing-code
Temporarily disable systemTest flag
fc846ab
@kevinmingtarja @all-seeing-code
Mark DockerCp for deprecation, add new DockerCpFromContainer
3bc2b61
@kevinmingtarja @all-seeing-code
Temporarily skip flaky package
9f9e41f
@kevinmingtarja @all-seeing-code
Add tmp debugging for flaky tests
9c1f0ab
@kevinmingtarja @all-seeing-code
Minor refactor
d775508
@kevinmingtarja @all-seeing-code
Remove systemTest flag from main_test
875e2ac
@kevinmingtarja @all-seeing-code
Fix stderr debugging
1de5406
@kevinmingtarja @all-seeing-code
Remove systemTest flag in poorman_auth docker-compose
bf8732b
@kevinmingtarja @all-seeing-code
Fix docker network not being removed
deed061
@kevinmingtarja @all-seeing-code
Use old format in poorman_auth docker-compose
524d9fd
@dshekhar95 @all-seeing-code
add test coverage to docker compose
b8fba79
@dshekhar95 @all-seeing-code
fix command
6813089
@dshekhar95 @all-seeing-code
remove flag from load tests
e8ae822
@skrdgraph
Copy link
Contributor

I think there is an issue with the compose file somewhere ... the first ACL curl test is failing. I will try to make a fix on this PR branch shortly.

@all-seeing-code
Dont run coverage with tests
94b8864 
(cherry picked from commit 9e12585)
@all-seeing-code
Copy link
Contributor Author

I think there is an issue with the compose file somewhere ... the first ACL curl test is failing. I will try to make a fix on this PR branch shortly.

No, issue was in workflow where I forgot to remove ./t --coverage=true from dgraph tests workflow file. Since dgraph tests doesn't build a coverage enabled binary, it wasn't able to understand the flags being passed from the environment variable and the cluster wasn't coming up.

See the logs:

Docker logs for c434568337e84f2780492df1893915e2be1c6c4ae9f9bdc13d704ba9426b585c is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 3b6a975a5921586f4a1e4c963da27b17931609c504ff15b27b889ea6519901a9 is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 67f4bc52c73957587e4fbbe14c85e2aa69333f8b165be6dd07a4f566cd6a668c is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 9e5834bbada3fbbf62f67297f9f6585b3d011172e430f1741d2e6caee80991d5 is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 18e4b7dfdae752fd0ef66628aeee996375be0447865ffd852eda52d1f5c3bb62 is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 6fc196e542cd91fb0da5cb8f663e7d7f8717bd09a4010682e17417d0f10fbd46 is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 69feab368bd84b2f7a476bdb338a9f09ec8792540811b0411f7614738371f36d is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for c524e335656c7d04319f703a503b17a2358179faadb2485e3d78042539c29f7e is Error: unknown flag: --test.coverprofile
 with error <nil> Docker logs for 14777bcb4ca266bb7aa75efd9960c19c5b9901a429e14fd3a0359d6966dc13b1 is Error: unknown flag: --test.coverprofile
 with error <nil> ---> LOGS for test-757-1 written to ./test-757-169752865.log .

@all-seeing-code all-seeing-code marked this pull request as ready for review December 9, 2022 11:59
@meghalims meghalims self-requested a review December 9, 2022 16:35
meghalims
meghalims approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@meghalims meghalims left a comment

Choose a reason for hiding this comment

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

Changes look good. Two Tests are still in progress. Once tests pass, you can get this in

skrdgraph
skrdgraph reviewed Dec 9, 2022
View reviewed changes
dgraph/main_test.go Outdated Show resolved Hide resolved
@all-seeing-code all-seeing-code changed the title chore: Orchestrate test binary and docker compose files to report coverage for integration tests fix(chore): Orchestrate test binary and docker compose files to report coverage for integration tests Dec 9, 2022
skrdgraph
skrdgraph approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@skrdgraph skrdgraph left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this... @anurags92! This is great.

dshekhar95
dshekhar95 approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@dshekhar95 dshekhar95 left a comment

Choose a reason for hiding this comment

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

lgtm

@skrdgraph
Copy link
Contributor

https://coveralls.io/jobs/111100282 <- this is where the results

@skrdgraph skrdgraph merged commit 035f9fc into main Dec 9, 2022
@skrdgraph skrdgraph deleted the anurag/system-test-coverage branch December 9, 2022 20:26
@joshua-goldstein joshua-goldstein mentioned this pull request Dec 13, 2022
Merged
joshua-goldstein added a commit that referenced this pull request Dec 14, 2022
@joshua-goldstein
chore(ci): set up ci for 21.03-slash (#8469)
7dfac4e 
## Problem

Currently our CI runs against main. We would like to run CI against our
slash branch also.

## Solution

In order for CI to run on release/v21.03-slash we must bring over all
the latest changes we made to our main branch. This is work going back a
few months, hence the large number of changes. Going forward we should
keep CI/CD related changes on release branches and main in harmony.

## Steps:
```
git checkout release/v21.03-slash
git checkout main -- Makefile
git checkout main -- dgraph/Makefile
git checkout main -- .github
git checkout main -- .go-version
git checkout main -- t
git checkout main -- tlstest/mtls_internal/tls
git checkout main -- graphql/e2e/custom_logic/cmd/Dockerfile
git checkout main -- systest/ldbc
git rm .github/workflows/{main.yml,golanglintci-lint.yml,label.yml}
```

Bring in coverage changes from #8494 in testutil/{docker.go,bulk.go},
bring in updated dockercompose files (with coverage variable), and
coverage entrypoint

## Todo

#8277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dshekhar95 dshekhar95 dshekhar95 approved these changes

@skrdgraph skrdgraph skrdgraph approved these changes

@meghalims meghalims meghalims approved these changes

@joshua-goldstein joshua-goldstein Awaiting requested review from joshua-goldstein

@akon-dey akon-dey Awaiting requested review from akon-dey

@MichelDiz MichelDiz Awaiting requested review from MichelDiz

@matthewmcneely matthewmcneely Awaiting requested review from matthewmcneely

@darkn3rd darkn3rd Awaiting requested review from darkn3rd

@gcxml gcxml Awaiting requested review from gcxml

@billprovince billprovince Awaiting requested review from billprovince

Assignees
No one assigned
Labels
area/graphql Issues related to GraphQL support on Dgraph. area/integrations Related to integrations with other projects. area/testing Testing related issues
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@all-seeing-code @coveralls @skrdgraph @meghalims @dshekhar95 @kevinmingtarja