Skip to content

fix(controller): TestPodExists unit test#6763

Merged
alexec merged 1 commit intoargoproj:masterfrom
tczhao:feature/fix-unit-test-TestPodExists
Sep 20, 2021
Merged

fix(controller): TestPodExists unit test#6763
alexec merged 1 commit intoargoproj:masterfrom
tczhao:feature/fix-unit-test-TestPodExists

Conversation

@tczhao
Copy link
Member

@tczhao tczhao commented Sep 18, 2021

Recent PR #6712 Added TestPodExists

the code is fine, runs well in local machine but somehow in github ci, it fails very often
#6753 https://github.com/argoproj/argo-workflows/runs/3632323865?check_suite_focus=true
#6749 https://github.com/argoproj/argo-workflows/runs/3627112929?check_suite_focus=true

I tested using

	assert.Equal(t, "hello-world", pod.Name)
	assert.Equal(t, "hello-world", pod.ObjectMeta.Name)
	assert.Equal(t, pod.Name, pod.ObjectMeta.Name)

They indeed show the same value in github CI

but using woc.podExists(pod.Name) just fails

changing to woc.podExists(pod.ObjectMeta.Name) seems to solve the issue

Tips:

  • Maybe add you organization to USERS.md.
  • Your PR needs to pass the required checks before it can be approved. If the check is not required (e.g. E2E tests) it does not need to pass
  • Sign-off your commits to pass the DCO check: git commit --signoff.
  • Run make pre-commit -B to fix codegen or lint problems.
  • Say how how you tested your changes. If you changed the UI, attach screenshots.
  • If changes were requested, and you've made them, then dismis the review to get it looked at again.
  • You can ask for help!

@tczhao tczhao force-pushed the feature/fix-unit-test-TestPodExists branch from b59abf8 to ab49095 Compare September 18, 2021 01:23
@codecov
Copy link

codecov bot commented Sep 18, 2021

Codecov Report

Merging #6763 (41e8236) into master (7684ef4) will decrease coverage by 0.03%.
The diff coverage is n/a.

❗ Current head 41e8236 differs from pull request most recent head deba156. Consider uploading reports for the commit deba156 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6763      +/-   ##
==========================================
- Coverage   48.64%   48.60%   -0.04%     
==========================================
  Files         264      264              
  Lines       19076    19076              
==========================================
- Hits         9279     9272       -7     
- Misses       8752     8762      +10     
+ Partials     1045     1042       -3     
Impacted Files Coverage Δ
server/workflow/workflow_server.go 44.41% <0.00%> (-2.40%) ⬇️
workflow/metrics/server.go 19.29% <0.00%> (+3.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7684ef4...deba156. Read the comment docs.

Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
@tczhao tczhao force-pushed the feature/fix-unit-test-TestPodExists branch 4 times, most recently from d1198d7 to deba156 Compare September 18, 2021 02:30
@tczhao tczhao marked this pull request as ready for review September 18, 2021 02:42
@alexec alexec merged commit 1a76e65 into argoproj:master Sep 20, 2021
@alexec alexec linked an issue Sep 21, 2021 that may be closed by this pull request
@sarabala1979 sarabala1979 mentioned this pull request Sep 28, 2021
36 tasks
kriti-sc pushed a commit to kriti-sc/argo-workflows that referenced this pull request Oct 24, 2021
Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
Signed-off-by: kriti-sc <kathuriakriti1@gmail.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.

TestGetPodByNode is flakey

2 participants