-
Notifications
You must be signed in to change notification settings - Fork 9.2k
YARN-11645. Order the queues by queuePath in json assert tests. #6432
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
Conversation
Change-Id: Iff7c8d0522003f9ae9d5c01a1070158e45903d3c
0ebe0e2 to
78565a1
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Change-Id: Ia848a19b0d2cb9659b34caa3654888c0fa18a374
Change-Id: I64470d2e28060204e5356432f461041fe732e540
a capacity-scheduler.xml, depending on the test order test cases were failing due to this Change-Id: I2ef59b12b5a1d02efb905eea1e95dd4ea9942400
...r/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestWebServiceUtil.java
Outdated
Show resolved
Hide resolved
|
Thanks @tomicooler for the change! I have just one small question |
|
💔 -1 overall
This message was automatically generated. |
brumi1024
left a comment
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.
Thanks @tomicooler for the patch. Commented.
| IOException { | ||
| assertJsonType(response); | ||
| JSONObject json = response.getEntity(JSONObject.class); | ||
| sortQueuesLexically(json); |
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.
I was meant to ask this before: is there a reason we're using two separate Json libraries to do (in the end) a string comparison? Couldn't we move completely to Jackson?
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.
Moved to jackson. There was a change in the test data, because jettison converted the floats (e.g. capacity=100.0) to an int (capacity=100).
Change-Id: Icd2e08eb93f9b8a75fb81bc7ebf282974fc52ca8
Change-Id: I7876fb2f11c5ab1b4fd6083542d5a6faa399a936
Change-Id: I23290e1cddec49b8016b0525d3ea0f4ed127c871
Change-Id: I2998e4028fc34e9adbe2112d8a1e4ccd792e3b41
|
💔 -1 overall
This message was automatically generated. |
Change-Id: I422e8ffeed4253f72d85069a7e79544c43a2447e
Change-Id: I30fbe959b1a5e0fd9b5cc8ee8b9b62b9bfdb8e1d
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Change-Id: I972ad9ae54256849965aa99187d88fe336696b65
brumi1024
left a comment
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.
Thanks @tomicooler, the latest state LGTM.
dineshchitlangia
left a comment
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
|
🎊 +1 overall
This message was automatically generated. |
|
Thanks @tomicooler for the patch, @K0K0V0K , @dineshchitlangia for the review, merging to trunk. |
… tests. (apache#6432) Change-Id: Iea7eaa34b1f24f863c701eb6c31304ed8dc7bce8 (cherry picked from commit 965b8975e87934385e8f3ddc8436adbfbca9aad2) (cherry picked from commit 97a56d70008fc5683cae202bb90069b7e52029cc)
Change-Id: Iff7c8d0522003f9ae9d5c01a1070158e45903d3c
Description of PR
The JSON assert tests that were using the
createMutableRMwere flaky the queue order changed in the "queue": [ {}, {} ] lists. We had a workaround likereinitAfterNodeChane(with a typo) but it didn't work well.Since there is a comment in the production code in
CapacitySchedulerInfo.getQueuesabout a marshalling issue,I didn't introduce sorting there, just here for the tests.
git grep -rn '"queuePath" :'How was this patch tested?
Ran the unit tests locally. YETUS will verify it.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?