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

feat: adding gocover to e2e tests (#18130) #18247

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

rumstead
Copy link
Contributor

@rumstead rumstead commented May 16, 2024

closes #18130

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
@rumstead rumstead changed the title WIP feat: adding gocover to e2e tests feat: adding gocover to e2e tests (#18130) May 16, 2024
@rumstead rumstead marked this pull request as ready for review May 16, 2024 14:43
@rumstead rumstead requested review from a team as code owners May 16, 2024 14:43
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
@crenshaw-dev
Copy link
Collaborator

Thanks for the PR! Can you rebase? Codecov report uploading was broken.

…de-coverage

# Conflicts:
#	.github/workflows/ci-build.yaml
@rumstead
Copy link
Contributor Author

Thanks for the PR! Can you rebase? Codecov report uploading was broken.

Done! However, I am not 100% sure how to test this. I have a feeling this won't work the first time.

Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.90%. Comparing base (c99fd49) to head (bb54a89).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #18247      +/-   ##
==========================================
- Coverage   44.90%   44.90%   -0.01%     
==========================================
  Files         354      354              
  Lines       47704    47705       +1     
==========================================
- Hits        21423    21421       -2     
- Misses      23486    23488       +2     
- Partials     2795     2796       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@crenshaw-dev
Copy link
Collaborator

Looks like codecov is uploading, but the coverage amount hasn't changed... will look tomorrow

@rumstead
Copy link
Contributor Author

rumstead commented Jun 3, 2024

Looks like codecov is uploading, but the coverage amount hasn't changed... will look tomorrow

Run go tool covdata percent -i=./test-results -o test-results/full-coverage.out
warning: no applicable files found in input directories

Checking https://github.com/argoproj/argo-cd/actions/runs/9341278349/job/25707896658?pr=18247 and the uploading e2e coverage report

Run actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808
Warning: No files were found with the provided path: /test-results/cov*. No artifacts will be uploaded.

I think I had a bad path

Procfile Outdated
@@ -1,13 +1,13 @@
controller: [ "$BIN_MODE" = 'true' ] && COMMAND=./dist/argocd || COMMAND='go run ./cmd/main.go' && sh -c "HOSTNAME=testappcontroller-1 FORCE_LOG_COLORS=1 ARGOCD_FAKE_IN_CLUSTER=true ARGOCD_TLS_DATA_PATH=${ARGOCD_TLS_DATA_PATH:-/tmp/argocd-local/tls} ARGOCD_SSH_DATA_PATH=${ARGOCD_SSH_DATA_PATH:-/tmp/argocd-local/ssh} ARGOCD_BINARY_NAME=argocd-application-controller $COMMAND --loglevel debug --redis localhost:${ARGOCD_E2E_REDIS_PORT:-6379} --repo-server localhost:${ARGOCD_E2E_REPOSERVER_PORT:-8081} --otlp-address=${ARGOCD_OTLP_ADDRESS} --application-namespaces=${ARGOCD_APPLICATION_NAMESPACES:-''} --server-side-diff-enabled=${ARGOCD_APPLICATION_CONTROLLER_SERVER_SIDE_DIFF:-'false'}"
api-server: [ "$BIN_MODE" = 'true' ] && COMMAND=./dist/argocd || COMMAND='go run ./cmd/main.go' && sh -c "FORCE_LOG_COLORS=1 ARGOCD_FAKE_IN_CLUSTER=true ARGOCD_TLS_DATA_PATH=${ARGOCD_TLS_DATA_PATH:-/tmp/argocd-local/tls} ARGOCD_SSH_DATA_PATH=${ARGOCD_SSH_DATA_PATH:-/tmp/argocd-local/ssh} ARGOCD_BINARY_NAME=argocd-server $COMMAND --loglevel debug --redis localhost:${ARGOCD_E2E_REDIS_PORT:-6379} --disable-auth=${ARGOCD_E2E_DISABLE_AUTH:-'true'} --insecure --dex-server http://localhost:${ARGOCD_E2E_DEX_PORT:-5556} --repo-server localhost:${ARGOCD_E2E_REPOSERVER_PORT:-8081} --port ${ARGOCD_E2E_APISERVER_PORT:-8080} --otlp-address=${ARGOCD_OTLP_ADDRESS} --application-namespaces=${ARGOCD_APPLICATION_NAMESPACES:-''}"
controller: [ "$BIN_MODE" = 'true' ] && COMMAND=./dist/argocd || COMMAND='go run ./cmd/main.go' && sh -c "GOCOVERDIR="test-results" HOSTNAME=testappcontroller-1 FORCE_LOG_COLORS=1 ARGOCD_FAKE_IN_CLUSTER=true ARGOCD_TLS_DATA_PATH=${ARGOCD_TLS_DATA_PATH:-/tmp/argocd-local/tls} ARGOCD_SSH_DATA_PATH=${ARGOCD_SSH_DATA_PATH:-/tmp/argocd-local/ssh} ARGOCD_BINARY_NAME=argocd-application-controller $COMMAND --loglevel debug --redis localhost:${ARGOCD_E2E_REDIS_PORT:-6379} --repo-server localhost:${ARGOCD_E2E_REPOSERVER_PORT:-8081} --otlp-address=${ARGOCD_OTLP_ADDRESS} --application-namespaces=${ARGOCD_APPLICATION_NAMESPACES:-''} --server-side-diff-enabled=${ARGOCD_APPLICATION_CONTROLLER_SERVER_SIDE_DIFF:-'false'}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need to make the same changes to the test tool Procfile?

@rumstead
Copy link
Contributor Author

rumstead commented Jun 3, 2024

Didn't seem to help. When I run:

make install-test-tools-local
make start-e2e-local
make test-e2e-local

I see the test-results directory populated

❯ ls  test-results/cov* 
test-results/covcounters.59ea707707b901d5957baf3fc0fa64f0.12281.1715805185934105000 test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.40502.1717427615343989000
test-results/covcounters.59ea707707b901d5957baf3fc0fa64f0.12294.1715805187074782000 test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.40504.1717427620834574000
test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.39184.1717427697662708000 test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.40506.1717427621139324000
test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.39186.1717427417033107000 test-results/coverage.out
test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.40499.1717427617518722000 test-results/covmeta.59ea707707b901d5957baf3fc0fa64f0
test-results/covcounters.c6bb65ce0a5fa66569a4cb1fc6efc6b4.40500.1717427613681107000 test-results/covmeta.c6bb65ce0a5fa66569a4cb1fc6efc6b4

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
@rumstead
Copy link
Contributor Author

rumstead commented Jun 4, 2024

Ok after looking at the e2e-server.log I see the error for all the binaries.

19:20:11               repo-server | �[merror: coverage meta-data emit failed: output directory "test-results" inaccessible (err: stat test-results: no such file or directory); no coverage data written

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
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.

Collect test coverage from e2e tests
2 participants