2030917 - Use assertIs<Type>() instead of assertTrue(x is Type) in tests

Status: RESOLVED FIXED

(Firefox for Android :: Tooling, enhancement)

Description

[Marcin Koziński]

assertIs<SomeType>(x) from kotlin.test gives a smart cast on the variable and a descriptive error on failure. Add a lint check with a quick-fix that will guide developers towards this nicer API, by showing a warning in the IDE complete with a "quick-fix" to replace code with one click. Also replace all existing usages of assertTrue(x is Type) in fenix and android-components tests.

Comment 1

[Marcin Koziński]

Attachment: Bug 2030917 - Use assertIs<Type>() instead of assertTrue(x is Type) in Fenix tests

Comment 2

[Marcin Koziński]

Attachment: Bug 2030917 - Use assertIs<Type>() instead of assertTrue(x is Type) in android-components

Comment 3

[Marcin Koziński]

Attachment: Bug 2030917 - Add an Android Lint check for assertTrue(x is Type) and a quick-fix to replace with assertIs<Type>()

Comment 4

[Marcin Koziński]

Attachment: Bug 2030917 - Use assertIs<Type>() instead of assertTrue(x is Type) in Focus tests

Comment 5

[Marcin Koziński]

Attachment: Bug 2030917 - Use assertIs<Type>() instead of assertTrue(x is Type) in GeckoView tests

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2d9ae4b9de28
https://hg.mozilla.org/integration/autoland/rev/ecda386de4e1
Use assertIs<Type>() instead of assertTrue(x is Type) in Fenix tests r=android-reviewers,nalexander,twhite,sfamisa
https://github.com/mozilla-firefox/firefox/commit/e46c56185971
https://hg.mozilla.org/integration/autoland/rev/8e888baf2818
Use assertIs<Type>() instead of assertTrue(x is Type) in android-components r=android-reviewers,nalexander,sfamisa
https://github.com/mozilla-firefox/firefox/commit/87d935f6121d
https://hg.mozilla.org/integration/autoland/rev/7f08d037b646
Use assertIs<Type>() instead of assertTrue(x is Type) in Focus tests r=android-reviewers,mcarare
https://github.com/mozilla-firefox/firefox/commit/65a0823890d8
https://hg.mozilla.org/integration/autoland/rev/21a94c445834
Use assertIs<Type>() instead of assertTrue(x is Type) in GeckoView tests r=geckoview-reviewers,ohall
https://github.com/mozilla-firefox/firefox/commit/f55dad5b2c83
https://hg.mozilla.org/integration/autoland/rev/99d780f7488e
Add an Android Lint check for assertTrue(x is Type) and a quick-fix to replace with assertIs<Type>() r=android-reviewers,nalexander,pollymce

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ecda386de4e1
https://hg.mozilla.org/mozilla-central/rev/8e888baf2818
https://hg.mozilla.org/mozilla-central/rev/7f08d037b646
https://hg.mozilla.org/mozilla-central/rev/21a94c445834
https://hg.mozilla.org/mozilla-central/rev/99d780f7488e

Comment 8

[Marcin Koziński]

Attachment: Bug 2030917 - Downgrade AssertIsDetector to warning in android-components

AssertIsDetector was always meant to only be a warning with a quick-fix, so it helps developers find their way to the recommended API without annoying them with backouts. The original patch missed the additional configuration applied in android-components that upgrades all warnings to errors.

Comment 9

[Marcin Koziński]

Attachment: Bug 2030917 - Allow lint to import assertIs when applying the quick-fix

Comment 10

[Marcin Koziński]

Attachment: Bug 2030917 - Simplify redundant skipParenthesizedExprDown in AssertIsDetector

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/95f062277259
https://hg.mozilla.org/integration/autoland/rev/b6996a111698
Downgrade AssertIsDetector to warning in android-components r=android-reviewers,gmalekpour,twhite
https://github.com/mozilla-firefox/firefox/commit/b4956d72d510
https://hg.mozilla.org/integration/autoland/rev/e995ff3b4a13
Allow lint to import assertIs when applying the quick-fix r=android-reviewers,twhite
https://github.com/mozilla-firefox/firefox/commit/19105e0bd868
https://hg.mozilla.org/integration/autoland/rev/5a1ee61980ce
Simplify redundant skipParenthesizedExprDown in AssertIsDetector r=android-reviewers,gmalekpour

Comment 13

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/b6996a111698
https://hg.mozilla.org/mozilla-central/rev/e995ff3b4a13
https://hg.mozilla.org/mozilla-central/rev/5a1ee61980ce