[YUNIKORN-1649] Use Junit reporter in all suites #561
Conversation
|
wait with kicking off the workflow until YUNIKORN-1666 is fixed |
test/e2e/admission_controller/admission_controller_suite_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Some copy-paste issues.
Also, pls check the naming of the xml files for consistency, other suites use names like "TEST-resourcefairness.xml", "TEST-bin_packing_junit.xml", but not every suite follows this. This could potentially be addressed in a follow-up JIRA.
Codecov Report
@@ Coverage Diff @@
## master #561 +/- ##
==========================================
- Coverage 69.97% 69.93% -0.05%
==========================================
Files 46 47 +1
Lines 7881 7925 +44
==========================================
+ Hits 5515 5542 +27
- Misses 2163 2176 +13
- Partials 203 207 +4 see 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
@wusamzong Please address the comments raised by @pbacsko. Every test suite should have appropriate names (in 2 places).
Also, other scope of this jira is to explore "Does Junit reporter library itself provide any options to create the directory if it doesn't exists?". This helps to avoid creating directory in Makefile. Having hard coded directory name itself in Makefile is error prone. So we need to see if there is a way to avoid it.
There was a problem hiding this comment.
@wusamzong Please address the comments above, the other parts is ok to me.
|
@pbacsko , @manirajv06 , @HuangTing-Yao Thanks for your review! I will make the change. |
|
The Ginkgo file only mentions creating a temporary directory. |
| report, | ||
| filepath.Join(configmanager.YuniKornTestConfig.LogDir, "QueueQuotaMgmt_junit.xml"), | ||
| filepath.Join(configmanager.YuniKornTestConfig.LogDir, "TEST-QueueQuotaMgmt_junit.xml"), |
There was a problem hiding this comment.
Should be TEST-queue_quota_mgmt_junit.xml
| @@ -52,7 +52,9 @@ var _ = AfterSuite(func() { | |||
|
|
|||
| func TestResourceFairness(t *testing.T) { | |||
| ginkgo.ReportAfterSuite("Resource Fairness", func(report ginkgo.Report) { | |||
There was a problem hiding this comment.
Should be TestResourceFairness.
| err := reporters.GenerateJUnitReportWithConfig( | ||
| err := common.CreateJUnitReportDir() | ||
| Ω(err).NotTo(gomega.HaveOccurred()) | ||
| err = reporters.GenerateJUnitReportWithConfig( | ||
| report, | ||
| filepath.Join(configmanager.YuniKornTestConfig.LogDir, "TEST-resourcefairness.xml"), |
There was a problem hiding this comment.
Should be TEST-resource_fairness_junit.xml.
| err := reporters.GenerateJUnitReportWithConfig( | ||
| err := common.CreateJUnitReportDir() | ||
| Ω(err).NotTo(gomega.HaveOccurred()) | ||
| err = reporters.GenerateJUnitReportWithConfig( | ||
| report, | ||
| filepath.Join(configmanager.YuniKornTestConfig.LogDir, "TEST-SparkJobs_junit.xml"), |
There was a problem hiding this comment.
Should be TEST-spark_jobs_junit.xml.
| report, | ||
| filepath.Join(configmanager.YuniKornTestConfig.LogDir, "StateAwareAppScheduling_junit.xml"), | ||
| filepath.Join(configmanager.YuniKornTestConfig.LogDir, "TEST-StateAwareAppScheduling_junit.xml"), |
There was a problem hiding this comment.
Should be TEST-stateaware_app_scheduling_junit.xml.
| @@ -52,9 +52,11 @@ var _ = AfterSuite(func() { | |||
|
|
|||
| func TestStateAwareAppScheduling(t *testing.T) { | |||
There was a problem hiding this comment.
Method should be named TestQueueQuotaMgmt.
|
@craigcondit Thanks for your review! I solved the name issues you mentioned |
What is this PR for?
Using Junit in all test suites.
Create a conditional statement to exclude the case that the test directory(/tmp/e2e-test-reports) has been created by
make e2e_testWhat type of PR is it?
What is the Jira issue?
How should this be tested?
Screenshots (if appropriate)
Questions: