Skip to content
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

[TextFields] Invalidate caret timers in snapshot tests. #6182

Merged
merged 1 commit into from Jan 4, 2019
Merged

[TextFields] Invalidate caret timers in snapshot tests. #6182

merged 1 commit into from Jan 4, 2019

Conversation

romoore
Copy link
Contributor

@romoore romoore commented Jan 4, 2019

Text fields snapshot tests were taking longer and longer to execute. The root
cause was "caret blink" timers added to the main run loop for every text field
that was set to isEditing = YES. It is sufficient to remove all subviews of
the text field from their superviews to invalidate the timer. The actual
culprit appears to be a private subview of class UITextSelectionView, but
since accessing it would be more fragile than this solution, it's best to just
destroy the view hierarchy.

Execution time with this change

	 Executed 6 tests, with 0 failures (0 unexpected) in 0.200 (0.202) seconds
	 Executed 12 tests, with 0 failures (0 unexpected) in 0.132 (0.137) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.206 (2.220) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.935 (1.950) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.097 (2.111) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.969 (1.983) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.903 (1.918) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.811 (1.825) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.269 (2.284) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.953 (1.967) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.383 (2.398) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.062 (2.076) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.953 (1.967) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.110 (2.124) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.723 (1.737) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.144 (2.158) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.051 (2.065) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.757 (1.771) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.729 (1.743) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.858 (1.872) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.258 (2.272) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.884 (1.898) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.292 (2.306) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.205 (2.219) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.928 (1.942) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.903 (1.917) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.692 (1.706) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.040 (2.054) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.729 (1.744) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.094 (2.108) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.936 (1.951) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.769 (1.783) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 1.772 (1.786) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.369 (2.383) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.944 (2.958) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.449 (2.463) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.882 (2.896) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.858 (2.872) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.567 (2.581) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.518 (2.532) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.669 (2.683) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.128 (2.142) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.479 (2.494) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.105 (2.119) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.518 (2.532) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.502 (2.516) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.171 (2.185) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.149 (2.163) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.730 (2.744) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.361 (2.375) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.758 (2.773) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.718 (2.732) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.424 (2.439) seconds
	 Executed 39 tests, with 0 failures (0 unexpected) in 2.382 (2.396) seconds
	 Executed 2046 tests, with 0 failures (0 unexpected) in 114.427 (115.196) seconds
	 Executed 0 tests, with 0 failures (0 unexpected) in 0.000 (0.000) seconds
	 Executed 2046 tests, with 0 failures (0 unexpected) in 114.427 (115.203) seconds

Closes #6181

Text fields snapshot tests were taking longer and longer to execute. The root
cause was "caret blink" timers added to the main run loop for every text field
that was set to `isEditing = YES`. It is sufficient to remove all subviews of
the text field from their superviews to invalidate the timer. The actual
culprit appears to be a private subview of class `UITextSelectionView`, but
since accessing it would be more fragile than this solution, it's best to just
destroy the view hierarchy.

Closes #6181
@romoore romoore added type:Internal cleanup Refactoring work is required. type:Test A test must be written. [TextFields] labels Jan 4, 2019
@romoore romoore removed the request for review from andrewoverton January 4, 2019 17:54
Copy link
Contributor

@wenyuzhang666 wenyuzhang666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. It seems surprising that the timer invalidation happens when UITextSelectionView gets explicitly removed from its superview but not in its dealloc process.

@romoore
Copy link
Contributor Author

romoore commented Jan 4, 2019

I can't say that I understand the reasoning exactly, but i believe that the objects aren't being deallocated immediately because of some weak references hanging around. In an actual product (like the Catalog), the objects are deallocated pretty quickly (when the VC deallocates), so I suspect this is a symptom of the way XCTestCase is handled.

We spoke offline and this approach is easier to understand (from a coding perspective), doesn't rely on significant knowledge of UITextField internals, and was more performant (by about 15%) than invalidating the timers directly.

@romoore romoore merged commit 3650061 into material-components:develop Jan 4, 2019
@romoore romoore deleted the issue-5762-f branch November 13, 2019 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes [TextFields] type:Internal cleanup Refactoring work is required. type:Test A test must be written.
Projects
None yet
3 participants