Sorted Strings to Fix test flakiness in testHumanPrinterAll #1

kavvya97 · 2023-10-09T18:07:06Z

Setup:
Java version: openjdk 11.0.20.1
Maven version: Apache Maven 3.6.3

Description of PR

The test org.apache.hadoop.mapreduce.jobhistory.TestHistoryViewerPrinter#testHumanPrinterAll can fail due to flakiness. These flakiness occurs because the test utilizes Hashmaps values and converts the values to string to perform the comparision and the order of the objects returned may not be necessarily maintained. This can be detected by utilizing the Nondex plugin.

Steps to reproduce:

git clone https://github.com/apache/hadoop
mvn install -pl hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core -am -DskipTests
Run the tests
mvn -pl hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core test -Dtests=org.apache.hadoop.mapreduce.jobhistory.TestHistoryViewerPrinter#testHumanPrinterAll
Run the test with the Nondex tool and observe the test results
mvn -pl hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.hadoop.mapreduce.jobhistory.TestHistoryViewerPrinter#testHumanPrinterAll
- Test Fails when Running Nondex in ONE mode (Assumes deterministic implementation of code but shuffled once different from underlying implementation) -DnondexMode=ONE & FULL Mode -DnondexMode=FULL (shuffles differently for each call)

For code changes:

The test utilizes Hashmap for storing job information and builts the string using HashMap.values(). However, the order of the objects returned by a Hashmap may not be maintained. Hence the test fails due to the string comparision. The following error occurs

testHumanPrinterAll(org.apache.hadoop.mapreduce.jobhistory.TestHistoryViewerPrinter)  Time elapsed: 0.297 s  <<< FAILURE!
org.junit.ComparisonFailure:
expected:<...8501754_0001_m_00000[7	6-Oct-2011 19:15:09	6-Oct-2011 19:15:16 (7sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000006	6-Oct-2011 19:15:08	6-Oct-2011 19:15:14 (6sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000005	6-Oct-2011 19:15:07	6-Oct-2011 19:15:12 (5sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000004	6-Oct-2011 19:15:06	6-Oct-2011 19:15:10 (4sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000003	6-Oct-2011 19:15:05	6-Oct-2011 19:15:08 (3sec)

SUCCEEDED REDUCE task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error
====================================================
task_1317928501754_0001_r_000008	6-Oct-2011 19:15:10	6-Oct-2011 19:15:18 (8sec)

SUCCEEDED JOB_CLEANUP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error
====================================================
task_1317928501754_0001_c_000009	6-Oct-2011 19:15:11	6-Oct-2011 19:15:20 (9sec)

JOB_SETUP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	HostName	Error	TaskLogs
====================================================
attempt_1317928501754_0001_s_000001_1	6-Oct-2011 19:15:03	6-Oct-2011 19:15:04 (1sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_s_000001_1

MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	HostName	Error	TaskLogs
====================================================
attempt_1317928501754_0001_m_000007_1	6-Oct-2011 19:15:09	6-Oct-2011 19:15:16 (7sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000007_1
attempt_1317928501754_0001_m_000002_1	6-Oct-2011 19:15:04	6-Oct-2011 19:15:06 (2sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000002_1
attempt_1317928501754_0001_m_000006_1	6-Oct-2011 19:15:08	6-Oct-2011 19:15:14 (6sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000006_1
attempt_1317928501754_0001_m_000005_1	6-Oct-2011 19:15:07	6-Oct-2011 19:15:12 (5sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000005_1
attempt_1317928501754_0001_m_000004_1	6-Oct-2011 19:15:06	6-Oct-2011 19:15:10 (4sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000004_1
attempt_1317928501754_0001_m_000003_1	6-Oct-2011 19:15:05	6-Oct-2011 19:15:08 (3sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000003]_1

REDUCE task list...> but was:<...8501754_0001_m_00000[5	6-Oct-2011 19:15:07	6-Oct-2011 19:15:12 (5sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000006	6-Oct-2011 19:15:08	6-Oct-2011 19:15:14 (6sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000004	6-Oct-2011 19:15:06	6-Oct-2011 19:15:10 (4sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000007	6-Oct-2011 19:15:09	6-Oct-2011 19:15:16 (7sec)

SUCCEEDED MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error	InputSplits
====================================================
task_1317928501754_0001_m_000003	6-Oct-2011 19:15:05	6-Oct-2011 19:15:08 (3sec)

SUCCEEDED REDUCE task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error
====================================================
task_1317928501754_0001_r_000008	6-Oct-2011 19:15:10	6-Oct-2011 19:15:18 (8sec)

SUCCEEDED JOB_CLEANUP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	Error
====================================================
task_1317928501754_0001_c_000009	6-Oct-2011 19:15:11	6-Oct-2011 19:15:20 (9sec)

JOB_SETUP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	HostName	Error	TaskLogs
====================================================
attempt_1317928501754_0001_s_000001_1	6-Oct-2011 19:15:03	6-Oct-2011 19:15:04 (1sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_s_000001_1

MAP task list for job_1317928501754_0001
TaskId		StartTime	FinishTime	HostName	Error	TaskLogs
====================================================
attempt_1317928501754_0001_m_000004_1	6-Oct-2011 19:15:06	6-Oct-2011 19:15:10 (4sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000004_1
attempt_1317928501754_0001_m_000005_1	6-Oct-2011 19:15:07	6-Oct-2011 19:15:12 (5sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000005_1
attempt_1317928501754_0001_m_000003_1	6-Oct-2011 19:15:05	6-Oct-2011 19:15:08 (3sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000003_1
attempt_1317928501754_0001_m_000007_1	6-Oct-2011 19:15:09	6-Oct-2011 19:15:16 (7sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000007_1
attempt_1317928501754_0001_m_000002_1	6-Oct-2011 19:15:04	6-Oct-2011 19:15:06 (2sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000002_1
attempt_1317928501754_0001_m_000006_1	6-Oct-2011 19:15:08	6-Oct-2011 19:15:14 (6sec)	localhost	http://t:1234/tasklog?attemptid=attempt_1317928501754_0001_m_000006]_1

Fix

The Fix involves extracting a single line of the string and sorting them to perform the string comparision. Since the strings are sorted, The comparision passes successfully. The Alternate way of sorting was converting to json but string being built is not json. Moreover, There was no library available to convert string to json.

How was this patch tested?

The fix was tested by adding a suitable fix and running the Nondex plugin again and ensuring that all the tests pass in FULL Mode and ONE Mode of the Nondex runs.

bbelide2 · 2023-10-12T17:41:21Z

Instead of mentioning "the test can fail", you can mention "test can fail due to flakiness" or "test becomes flaky"
You can show the exact error reported when NonDex is run. The error shows the difference between expected and actual values. It will clearly show the problem in the description.
Instead of adding code pointers as hyperlinks, you can add them as embedded which is easy to read. You can refer to the code links I added here
You can consider following this order which may be easier to understand - problem, root cause, fix, setup/how to test?
Did you consider changing HashMap to LinkedHashMap?
[Minor] Nondex hyperlink is not added properly.

krishnanand5 · 2023-10-12T21:13:43Z

Most of the changes I wanted to suggest have been suggested by @bbelide2. You can maybe add a line to indicate that the test execution has not been hindered by your changes.

kavvya97 · 2023-10-13T06:28:44Z

Instead of mentioning "the test can fail", you can mention "test can fail due to flakiness" or "test becomes flaky"

Fixed

You can show the exact error reported when NonDex is run. The error shows the difference between expected and actual values. It will clearly show the problem in the description.

The exact error is very Huge and it is a assertion error. Pasting the whole string would not be useful

Instead of adding code pointers as hyperlinks, you can add them as embedded which is easy to read. You can refer to the code links I added here

I am not sure how you can embed a link. I tried copy perma link but that didn't work. @bbelide2 can you tell me how to embed the code

You can consider following this order which may be easier to understand - problem, root cause, fix, setup/how to test?

The above template for the PR follows the hadoop default PR template.

Did you consider changing HashMap to LinkedHashMap?

Yes. However, HashMap is used at multiple places for constructing the string, Including the source code and the tests. The above fix address the tests without touch the source code.

[Minor] Nondex hyperlink is not added properly.

Fixed

bbelide2 · 2023-10-17T17:52:00Z

Looks good now. For embedding the code, you just have to select the code, copy permalink and paste the link in the description.

kavvya97 · 2023-10-17T18:49:16Z

Looks good now. For embedding the code, you just have to select the code, copy permalink and paste the link in the description.

Hey sukesh, I have done the same. unfortunately, I am not able to make the code be seen in the PR description

lxb007981 · 2023-11-03T04:38:37Z

This fix looks good and elegant to me. Using containsExactlyInAnyOrderElementsOf is a good idea.

harshith2000 · 2023-11-06T17:29:53Z

This fix looks good to me. Converting the string to a list and checking if they're the same (without the order) seems a good idea.

kavvya97 force-pushed the nondex-testHistoryViewPrinterAll branch 2 times, most recently from 0ce8ce9 to 32af584 Compare October 31, 2023 15:06

Fixed test flakiness during string comparision

777daa6

kavvya97 force-pushed the nondex-testHistoryViewPrinterAll branch from 32af584 to 777daa6 Compare October 31, 2023 19:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sorted Strings to Fix test flakiness in testHumanPrinterAll #1

Sorted Strings to Fix test flakiness in testHumanPrinterAll #1

kavvya97 commented Oct 9, 2023 •

edited

Loading

bbelide2 commented Oct 12, 2023

krishnanand5 commented Oct 12, 2023

kavvya97 commented Oct 13, 2023 •

edited

Loading

bbelide2 commented Oct 17, 2023

kavvya97 commented Oct 17, 2023

lxb007981 commented Nov 3, 2023

harshith2000 commented Nov 6, 2023

Sorted Strings to Fix test flakiness in testHumanPrinterAll #1

Are you sure you want to change the base?

Sorted Strings to Fix test flakiness in testHumanPrinterAll #1

Conversation

kavvya97 commented Oct 9, 2023 • edited Loading

Description of PR

For code changes:

Fix

How was this patch tested?

bbelide2 commented Oct 12, 2023

krishnanand5 commented Oct 12, 2023

kavvya97 commented Oct 13, 2023 • edited Loading

bbelide2 commented Oct 17, 2023

kavvya97 commented Oct 17, 2023

lxb007981 commented Nov 3, 2023

harshith2000 commented Nov 6, 2023

kavvya97 commented Oct 9, 2023 •

edited

Loading

kavvya97 commented Oct 13, 2023 •

edited

Loading