Add Test for duplicated test URLs, Fix #14659 #14660
Conversation
|
Any idea why you didn't pick up see https://github.com/EFForg/https-everywhere/pull/14740/files ( |
|
Ah, I'm guessing it's because it was whitelisted for coverage. |
@Bisaloo yes, whitelisted rulesets won't be showing up here. |
* Batch removal of duplicated test urls, related #14660 * Remove duplicated test urls from CloudAtCost.com.xml * Remove duplicated test urls from westberks.gov.uk.xml * Update Threema.xml, fix rules-test * Update Imgs_Mail.ru.xml, fix rules-test * Update Komoona.com.xml, fix rules-test * Update Wemfbox.ch.xml, fix rules-test * Update Twenga.com.xml, fix rules-test * Update Fenland.gov.uk.xml, fix rules-test * Update Destructoid.xml, fix fetch-test Remove store.destructoid.com, gone from dns * Update Komoona.com.xml, fix fetch-test * Update PropellerAds.xml, fix fetch-test Remove img.propellerads.com, gone from dns * Update Wemfbox.ch.xml, fix fetch-test Remove 20minro-ssl.wemfbox.ch, timeout Remove qs-ssl.wemfbox.ch, cert-chain Remove complicate rule for qs.wemfbox.ch * Update greenhouseci.com.xml, fix fetch-test Remove app.greenhouseci.com, mismatch * Update cloud66.com.xml, fix fetch-test Remove birdseye.cloud66.com, expired * Update WP-Engine.xml, fix fetch-test Remove complicated rules for press.wpengine.com, destination gone from dns * Update Hdfcbank.com.xml, fix fetch-test Remove imaging.hdfcbank.com, cert-chain Remove punjabadds.hdfcbank.com, cert-chain * Update JD.com-Problematic.xml Remove complicated rules for c-nfa.jd.com, obsolete * Update espivblogs.net.xml, fix fetch-test Remove eleftheriako-giro-giro.espivblogs.net, timeout * Update Twenga.com.xml, fix fetch-test * Update ruleset-whitelist.csv, fix fetch-test Fenland.gov.uk.xml, Wandoujia.xml, luosimao.com.xml, tanx.com.xml required modifications too complicated to be performed in this PR. * Update squat.gr.xml, fix fetch-test * Delete squat.gr.xml, part of #14944 * Update Hdfcbank.com.xml
|
ping @Hainish again |
| @@ -80,6 +80,12 @@ def __init__(self, url): | |||
| """ | |||
| self.url = url | |||
|
|
|||
| def __eq__(test1, test2): | |||
| return test1.__hash__() == test2.__hash__() | |||
There was a problem hiding this comment.
Why can't you use return test1.url == test2.url here, and remove the __hash__ method below? This would also remove the internal string __hash__ method, which is preferable.
|
|
||
| # check for duplicated test urls | ||
| if len(self.tests) != len(set(self.tests)): | ||
| for test in self.tests: |
There was a problem hiding this comment.
Currently, if you run this test it will print one error message for each time the duplicate test URL appears. You can avoid this by just making this line for test in set(self.tests):
|
@Hainish done. thanks!! |
| def __eq__(test1, test2): | ||
| return test1.url == test2.url | ||
|
|
||
| def __hash__(self): |
There was a problem hiding this comment.
We can get rid of this now, no?
|
|
||
| # check for duplicated test urls | ||
| if len(self.tests) != len(set(self.tests)): | ||
| uniqueTests = set(self.tests) |
There was a problem hiding this comment.
We're not using this anywhere. We should change the below line to be for test in set(self.tests):
|
@Hainish In 4c52886 (https://travis-ci.org/EFForg/https-everywhere/jobs/366387046) it seems that |
|
Thank you! |
UPDATE 2018.03.20 Blocked by #14944 Blocked by #14924
Close #14659 Close #14661 Hopefully before #14907
c18d192 seems to be faster than 2e3e2c1 over Travis...
We might want to go through some Alexa top sites before performing automatic removals.
... TODO LIST REMOVED ...