assertTrue should be used instead of comparing to true
Closed, DeclinedPublic
Actions

Assigned To

Authored By

	DannyS712
	Feb 7 2020, 1:22 AM

Description

https://codesearch.wmflabs.org/search/?q=assertSame%5C(%20true&i=nope&files=&repos=

assertSame( true, ... ) should be assertTrue( ... )

Details

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
Use assertTrue instead of assertSame to compare with true	mediawiki/extensions/CentralAuth	master	+21 -21
Add a sniff for using assertSame with null, true, or false	mediawiki/tools/codesniffer	master	+139 -1
Use assertTrue instead of assertSame to compare with true	mediawiki/extensions/OAuthAuthentication	master	+1 -1
Use assertTrue instead of assertSame to compare with true	mediawiki/services/parsoid	master	+6 -7
Use assertTrue instead of assertSame to compare with true	mediawiki/extensions/Popups	master	+1 -2
Use assertTrue instead of assertSame to compare with true	mediawiki/extensions/CirrusSearch	master	+2 -2

Customize query in gerrit

Related Objects

Mentioned In: T244556: add sniffs for using less specific assertions

Event Timeline

DannyS712 created this task.Feb 7 2020, 1:22 AM

Restricted Application added a project: User-DannyS712. · View Herald TranscriptFeb 7 2020, 1:22 AM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.Feb 7 2020, 1:23 AM

DannyS712 mentioned this in T244556: add sniffs for using less specific assertions.Feb 7 2020, 1:58 AM

Change 570794 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/OAuthAuthentication@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570794

Change 570795 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/CirrusSearch@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570795

Change 570796 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/CentralAuth@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570796

Change 570797 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/services/parsoid@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570797

DannyS712 moved this task from Next to In progress on the User-DannyS712 board.Feb 7 2020, 6:47 AM

Change 570803 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/Popups@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570803

Change 570795 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570795

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.19; 2020-02-11).Feb 7 2020, 10:00 AM

Change 570803 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570803

Change 570797 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Use assertTrue instead of assertSame to compare with true

https://gerrit.wikimedia.org/r/570797

Change 585973 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/tools/codesniffer@master] Add a sniff for using assertSame with null, true, or false

https://gerrit.wikimedia.org/r/585973

assertTrue should be used instead of comparing to true

What? No. Why?

From Gerrit:

The problem is with assertEqual, which performs lose/weak comparison. assertSame is fine and should not be sniffed for.

Change 585973 abandoned by DannyS712:
Add a sniff for using assertSame with null, true, or false

https://gerrit.wikimedia.org/r/585973

DannyS712 removed a project: User-DannyS712.May 27 2020, 8:27 AM

Change 570796 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/extensions/CentralAuth@master] Use assertTrue instead of assertSame to compare with true

Reason:

I forgot about this patch and accidentally did the change again as part of I4e0ff64. Sorry.