Bug 229788 - [webkitpy] Occasional exception thrown in change_result_to_failure when repeating flaky layout tests
Summary: [webkitpy] Occasional exception thrown in change_result_to_failure when repea...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lauro Moura
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-01 18:35 PDT by Lauro Moura
Modified: 2021-10-14 19:20 PDT (History)
4 users (show)

See Also:


Attachments
Tentative patch (4.44 KB, patch)
2021-09-01 18:44 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lauro Moura 2021-09-01 18:35:26 PDT
It's hard to reproduce with run-webkit-tests, but sometimes, when running multiple iterations of the same layout test, the run is aborted with the following backtrace (from running a single test with --iterations=100):

KeyError raised: 'fast/mediastream/RTCPeerConnection-iceconnectionstatechange-event.html'                                                                                                                                                     
Traceback (most recent call last):                                                                                                                                                                                                            
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 92, in main                                                                                                                                                
    run_details = run(port, options, args, stderr)                                                                                                                                                                                            
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 494, in run                                                                                                                                                
    run_details = manager.run(args)                                                                                                                                                                                                           
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 413, in run                                                                                                                                             
    temp_initial_results, temp_retry_results, temp_enabled_pixel_tests_in_retry = self._run_test_subset(test_inputs, device_type=device_type)                                                                                                 
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 487, in _run_test_subset                                                                                                                                
    initial_results = self._run_tests(test_inputs, self._options.repeat_each, self._options.iterations, int(self._options.child_processes), retrying=False, device_type=device_type)                                                          
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 566, in _run_tests               
    return self._runner.run_tests(self._expectations[device_type], new_test_inputs, num_workers, retrying, device_type)                                                                                                                       
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 175, in run_tests     
    pool.do(                                                                                                                                                                                                                                  
  File "/app/webkit/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/task_pool.py", line 397, in do                                                                                                                                          
    result = function(*args, **kwargs)                                                                                                                                                                                                        
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 76, in run_shard                                                                                                                             
    return Worker.instance.run_tests(shard)                                                                                                                                                                                                   
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 318, in run_tests                                                                                                                            
    Worker.instance.run_test(input, shard.name)                                                                                                                                                                                               
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 353, in run_test                                                                                                                             
    TaskPool.Process.queue.send(TaskPool.Task(                                                                                                                                                                                                
  File "/app/webkit/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/task_pool.py", line 165, in send                 
    object(None)                                                                                                       
  File "/app/webkit/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/task_pool.py", line 56, in __call__              
    return self.function(*self.args, **self.kwargs)                                                                    
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 81, in handle_finished_test                                                                                                                  
    LayoutTestRunner.instance.update_summary_with_result(result)                                                       
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 241, in update_summary_with_result                                                                                                           
    self._current_run_results.change_result_to_failure(existing, result, existing_expectation, expected)               
  File "/app/webkit/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py", line 102, in change_result_to_failure                                                                                                                    
    self.tests_by_expectation[existing_result.type].remove(existing_result.test_name)                                                                                                                                                         
KeyError: 'fast/mediastream/RTCPeerConnection-iceconnectionstatechange-event.html'

When this happens, a common sequence of events is:

* Unexpected results appears (e.g. Failure in an expected Timeout)
* Another kind of unexpected results appears (e.g. Pass in an expected Timeout)
* The same kind of unexpected results appears (e.g. Pass in an expected Timeout)
* EXCEPTION 

This seems to happen due to the `TestResults` instance stored in `TestRunResults.result_by_name` not being updated from Failure to Pass, as `TestResults.convert_to_failure` (from r235467) just extends the list of failures (IIUC, to keep existing failures information across test runs).

This causes the Failure test result to be removed from TestRunResults.test_by_expectation[Failure] inside change_result_to_failure, but still stored as the current result. In the next Pass result, the code tries to remove the Failure it again, raising KeyError.

Tentative patch in the next comment.
Comment 1 Lauro Moura 2021-09-01 18:44:14 PDT
Created attachment 437102 [details]
Tentative patch
Comment 2 Jonathan Bedard 2021-09-02 07:48:19 PDT
Comment on attachment 437102 [details]
Tentative patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437102&action=review

> Tools/ChangeLog:10
> +        Failure (likely due to r235467). This may cause change_result_to_failure

I would be inclined to blame 240653@main / r281216, but I don't think that effects the validity of this change.
Comment 3 Radar WebKit Bug Importer 2021-09-08 18:36:24 PDT
<rdar://problem/82902569>
Comment 4 Lauro Moura 2021-10-14 18:41:44 PDT
Comment on attachment 437102 [details]
Tentative patch

Patch still valid after testing locally.
Comment 5 EWS 2021-10-14 19:20:57 PDT
Committed r284227 (243036@main): <https://commits.webkit.org/243036@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437102 [details].