diff options
Diffstat (limited to 'app/src/test/java')
3 files changed, 296 insertions, 0 deletions
diff --git a/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt b/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt new file mode 100644 index 0000000..72dc73f --- /dev/null +++ b/app/src/test/java/org/terst/feedreader/data/CachedExtractionTest.kt @@ -0,0 +1,67 @@ +package org.terst.feedreader.data + +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The reported bug came with "extractedContent cached: true", which is the half + * that a fixed extractor cannot reach: the cookie disclaimer was already written + * to `articles.extractedContent`, and every open after that read it straight back + * out without ever looking at it again. + * + * So a poisoned cache has to be recognised and dropped on read, not just kept out + * on write. + */ +class CachedExtractionTest { + + private val cookieNotice = + "<div id=\"readability-page-1\" class=\"page\"><div id=\"didomi-host\"><p>This website uses " + + "cookies and other tracking technologies (“Cookies”) to enhance user experience, to " + + "display targeted ads, and to analyze performance and user activity on our website. For " + + "more information about our use of Cookies and our privacy practices, see our Privacy " + + "Policy. By continuing to use this website, you agree to our Terms and Conditions.</p>" + + "</div></div>" + + @Test + fun `real cached content is used as-is and the cache is left alone`() = runTest { + var cleared = false + val content = "<p>" + "The actual article body. ".repeat(60) + "</p>" + val result = loadCachedExtraction(content) { cleared = true } + assertEquals(content, result.content) + assertFalse(result.discardedConsentNotice) + assertFalse("a good cache must not be cleared", cleared) + } + + @Test + fun `a cached consent notice is discarded and deleted`() = runTest { + var cleared = false + val result = loadCachedExtraction(cookieNotice) { cleared = true } + assertNull("the notice must not be shown as the article", result.content) + assertTrue(result.discardedConsentNotice) + assertTrue("the poisoned row must be cleared so a re-extract can replace it", cleared) + } + + @Test + fun `nothing cached touches nothing`() = runTest { + var cleared = false + val result = loadCachedExtraction(null) { cleared = true } + assertNull(result.content) + assertFalse(result.discardedConsentNotice) + assertFalse(cleared) + } + + // A blank value is as good as nothing cached -- it is not a consent notice, so + // reporting it as a discarded one would make the caller re-extract on a false + // premise. + @Test + fun `a blank cached value is not reported as a discarded notice`() = runTest { + var cleared = false + val result = loadCachedExtraction(" ") { cleared = true } + assertFalse(result.discardedConsentNotice) + assertFalse(cleared) + } +} diff --git a/app/src/test/java/org/terst/feedreader/readability/ConsentNoticeTest.kt b/app/src/test/java/org/terst/feedreader/readability/ConsentNoticeTest.kt new file mode 100644 index 0000000..7f4e9cc --- /dev/null +++ b/app/src/test/java/org/terst/feedreader/readability/ConsentNoticeTest.kt @@ -0,0 +1,140 @@ +package org.terst.feedreader.readability + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Reported live: reader mode on a New Republic feature article showed nothing but + * the site's cookie disclaimer, and cached it. + * + * Reproduced outside the app by running the extractor's own injected script + * (ReaderExtractor's `script`, Readability.js and all) against the real page: when + * the article body is present, extraction is perfect. When the body is NOT in the + * DOM at the moment Readability runs, Readability's own fallback pass kicks in -- + * its first pass strips "unlikely candidates" (which the consent banner is, its + * class says `didomi-notice-banner`), comes back under its 500-character + * threshold, and so it retries with that stripping disabled. The Didomi notice's + * single paragraph is 510 characters, i.e. just over that threshold, so it is + * accepted as the article. + * + * [EXTRACTED_COOKIE_NOTICE] below is not a hand-written approximation of that -- + * it is the verbatim `content` string that reproduction produced. + * + * Nothing on the Kotlin side looked at that: any non-blank `content` counted as a + * successful extraction, got written to `articles.extractedContent`, and was never + * re-checked. Hence a permanently cached cookie disclaimer. + */ +class ConsentNoticeTest { + + @Test + fun `the reported New Republic extraction is recognised as a consent notice`() { + assertTrue(ConsentNotice.looksLikeConsentNotice(EXTRACTED_COOKIE_NOTICE)) + } + + // Readability strips class attributes but keeps ids, so the CMP container's id + // survives into the extracted HTML and is the most precise signal available. + @Test + fun `other consent-management platforms are recognised by their container id`() { + val cmpIds = listOf( + "didomi-host", + "onetrust-banner-sdk", + "onetrust-consent-sdk", + "CybotCookiebotDialog", + "qc-cmp2-container", + "sp_message_container_1234", + "truste-consent-track", + "usercentrics-root", + "osano-cm-dialog", + "iubenda-cs-banner", + "optanon-alert-box-wrapper", + "cookie-law-info-bar", + ) + for (id in cmpIds) { + val html = """<div id="readability-page-1"><div id="$id"><p>Please review your choices.</p></div></div>""" + assertTrue("$id should be recognised as a consent container", ConsentNotice.looksLikeConsentNotice(html)) + } + } + + // Not every cookie banner is a known CMP. A short result whose text is + // unmistakably consent boilerplate counts too, with no id to go on. + @Test + fun `a short unbranded cookie banner is recognised from its wording`() { + val html = """ + <div id="readability-page-1"><div><p>We use cookies and similar tracking technologies + to personalise content and ads. By continuing to use this site you consent to our use + of cookies. You can manage your preferences at any time.</p></div></div> + """.trimIndent() + assertTrue(ConsentNotice.looksLikeConsentNotice(html)) + } + + // One stray phrase is not enough -- plenty of ordinary writing mentions cookies. + @Test + fun `a single incidental phrase is not enough`() { + val html = """<div id="readability-page-1"><div><p>The recipe calls for cookies. + We use cookies in the frosting too.</p></div></div>""" + assertFalse(ConsentNotice.looksLikeConsentNotice(html)) + } + + // The whole point is to not throw away real articles. A genuine article ABOUT + // cookie-consent law would hit every phrase there is; length is what separates + // it from a banner. Nothing long is ever rejected. + @Test + fun `a long article about cookie consent is never rejected`() { + val paragraph = "<p>We use cookies and other tracking technologies, and by continuing " + + "to use this website you consent to our use of cookies -- that is the boilerplate " + + "the GDPR produced, and this article is about where it came from. You can manage " + + "your preferences, in theory.</p>" + val html = "<div id=\"readability-page-1\"><div>${paragraph.repeat(12)}</div></div>" + assertTrue("fixture must be past the length cap", html.length > 2_000) + assertFalse(ConsentNotice.looksLikeConsentNotice(html)) + } + + // Short posts are normal (kottke-style linkblogging is called out in + // ReaderExtractor's own comments) and must survive. + @Test + fun `an ordinary short post is not a consent notice`() { + val html = """ + <div id="readability-page-1"><div><p>A lovely short note about a bridge in Lisbon, + with one link and nothing else. This is the entire post.</p></div></div> + """.trimIndent() + assertFalse(ConsentNotice.looksLikeConsentNotice(html)) + } + + @Test + fun `null and blank content are not consent notices`() { + assertFalse(ConsentNotice.looksLikeConsentNotice(null)) + assertFalse(ConsentNotice.looksLikeConsentNotice("")) + assertFalse(ConsentNotice.looksLikeConsentNotice(" \n ")) + } + + // Markup must not count towards the length budget: a banner wrapped in enough + // nested divs and inline styles to exceed the cap in raw characters is still a + // banner. + @Test + fun `length is measured on text not markup`() { + val padding = "<div style=\"${"border-color: rgb(0, 0, 0); ".repeat(80)}\">" + val html = padding + EXTRACTED_COOKIE_NOTICE + "</div>" + assertTrue("fixture must be past the length cap in raw characters", html.length > 2_000) + assertTrue(ConsentNotice.looksLikeConsentNotice(html)) + } + + companion object { + /** + * Verbatim output of running ReaderExtractor's injected script against the + * reported page in a state where the article body was absent. Do not tidy + * this up -- its value is that it is exactly what the app produced. + */ + const val EXTRACTED_COOKIE_NOTICE: String = + "<div id=\"readability-page-1\" class=\"page\"><div id=\"didomi-host\" data-testid=\"notice\" " + + "data-nosnippet=\"true\" lang=\"en\" data-lang-dir=\"ltr\"><p>This website uses cookies and " + + "other tracking technologies (“Cookies”) to enhance user experience, to display " + + "targeted ads, and to analyze performance and user activity on our website. We also may " + + "share information about your activity on our website with our advertising and analytics " + + "providers. For more information about our use of Cookies and our privacy practices, see " + + "our <a href=\"https://newrepublic.com/pages/privacy\" target=\"_blank\">Privacy Policy</a>. " + + "By continuing to use this website, you agree to our " + + "<a href=\"https://newrepublic.com/pages/terms-and-conditions\" target=\"_blank\">Terms and " + + "Conditions</a>, including our dispute resolution provisions.</p></div></div>" + } +} diff --git a/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt b/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt new file mode 100644 index 0000000..8995e22 --- /dev/null +++ b/app/src/test/java/org/terst/feedreader/readability/EvalOutcomeTest.kt @@ -0,0 +1,89 @@ +package org.terst.feedreader.readability + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The decision ReaderExtractor makes about each `evaluateJavascript` result: + * accept it, inject again because the page probably was not ready, or give up + * with a reason. It used to live inline in a WebView callback, which meant the + * only way to exercise it was on a device against a live page. + * + * The case that mattered and was never covered: a result that is perfectly + * well-formed and non-blank, and is the page's cookie-consent banner rather than + * the article. That counted as a success, got cached, and stayed cached. + */ +class EvalOutcomeTest { + + private val maxAttempts = 6 + + private fun decide(article: ExtractedArticle?, attempt: Int, raw: String = "\"{}\"") = + decideEvalOutcome(article, raw, attempt, maxAttempts) + + private fun article(content: String?, error: String? = null) = + ExtractedArticle(title = "t", content = content, textContent = content, error = error) + + @Test + fun `real content is accepted on the first attempt`() { + val a = article("<p>" + "The actual article body. ".repeat(60) + "</p>") + val decision = decide(a, attempt = 1) + assertEquals(EvalDecision.Done(ExtractionResult.Success(a)), decision) + } + + // Retrying cannot change Readability's opinion of the markup, so a thrown + // error is final however early it arrives. + @Test + fun `a readability error is final immediately`() { + val decision = decide(article(null, error = "TypeError: x is null"), attempt = 1) + assertEquals( + EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.ReadabilityError("TypeError: x is null"))), + decision + ) + } + + @Test + fun `an undecodable result retries and then reports a bad result`() { + assertEquals(EvalDecision.Retry, decide(null, attempt = 1, raw = "null")) + val last = decide(null, attempt = maxAttempts, raw = "null") + assertEquals(EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.BadResult("null"))), last) + } + + @Test + fun `blank content retries and then reports no content`() { + assertEquals(EvalDecision.Retry, decide(article(""), attempt = 1)) + assertEquals( + EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.NoContent)), + decide(article(""), attempt = maxAttempts) + ) + } + + // A cookie banner is exactly what a half-loaded page yields -- the banner is + // injected early, the article body may not be in the DOM yet -- so the right + // first move is the retry the extractor already does for an empty result. + @Test + fun `a consent notice retries rather than being accepted`() { + val decision = decide(article(ConsentNoticeTest.EXTRACTED_COOKIE_NOTICE), attempt = 1) + assertEquals(EvalDecision.Retry, decision) + } + + @Test + fun `a consent notice that survives every attempt is reported as a failure`() { + val decision = decide(article(ConsentNoticeTest.EXTRACTED_COOKIE_NOTICE), attempt = maxAttempts) + assertEquals( + EvalDecision.Done(ExtractionResult.Failure(ExtractionFailure.ConsentInterstitial)), + decision + ) + } + + // The failure has to be distinguishable from "no content": one means the page + // has no article, the other means the page would not show it without consent. + @Test + fun `the consent failure says what happened`() { + val f: ExtractionFailure = ExtractionFailure.ConsentInterstitial + assertTrue(f.userMessage().isNotBlank()) + assertTrue(f.diagnostic().isNotBlank()) + assertTrue(f.userMessage().lowercase().contains("cookie") || f.userMessage().lowercase().contains("consent")) + assertTrue(f.userMessage() != ExtractionFailure.NoContent.userMessage()) + } +} |