-
Notifications
You must be signed in to change notification settings - Fork 134
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
[YUNIKORN-2714] Add e2e test to verify queue name with special characters #906
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #906 +/- ##
==========================================
+ Coverage 68.04% 68.20% +0.16%
==========================================
Files 70 70
Lines 7558 7606 +48
==========================================
+ Hits 5143 5188 +45
- Misses 2205 2211 +6
+ Partials 210 207 -3 ☔ View full report in Codecov by Sentry. |
return nil | ||
}) | ||
ginkgo.By("Fetch the queue information using the REST API") | ||
allQueues, err := restClient.GetQueues("default") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using this API and traverse over queues, it is better to directly hit Queue specific REST API https://yunikorn.apache.org/docs/api/scheduler#partition-queue because it covers the url encoding part as well. Please include the -ve test cases too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @manirajv06 , Updated the code with URL Encoded and Negative case as well. Please take a look.
} else { | ||
gomega.Ω(queue.QueueName).NotTo(gomega.Equal("root." + queueName)) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we started using direct API below, can we clean up the above code from line no. 664 to 674?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure will remove this part of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM.
This PR adds an end-to-end test to verify that a queue name with all allowed special characters is processed correctly. The test creates a queue with a name that includes all allowed special characters, fetches the queue information using the REST API, and verifies that the queue information is returned correctly. This is mainly to ensure no breakage in the REST API URL due to the special characters in the queue name.
What type of PR is it?
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-2714
How should this be tested?
Screenshots (if appropriate)
Questions: