Skip to content

Add the reference tests for the request blocklist functionality#7915

Merged
catalinradoiu merged 10 commits intodevelopfrom
feature/cradoiu/request-blocklist-reference-tests
Mar 12, 2026
Merged

Add the reference tests for the request blocklist functionality#7915
catalinradoiu merged 10 commits intodevelopfrom
feature/cradoiu/request-blocklist-reference-tests

Conversation

@catalinradoiu
Copy link
Contributor

@catalinradoiu catalinradoiu commented Mar 9, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/project/1211724162604201/task/1211938369145726?focus=true

Description

Adds comprehensive reference tests for the Request Blocklist feature to validate blocking behavior against standardized test cases. The test suite covers basic blocking functionality, allowlisting interactions, tracker detection integration, incorrect rule handling, and rule ordering scenarios.

Steps to test this PR

Request Blocklist Reference Tests

  • Run the new RequestBlocklistReferenceTest to verify all test cases pass
  • CI pases

UI changes

No UI changes


Note

Low Risk
Test-only changes that add a new instrumented reference test suite and accompanying fixtures; no production logic is modified.

Overview
Adds a new Android instrumented, parameterized reference test (RequestBlocklistReferenceTest) that exercises WebViewRequestInterceptor request-blocklist decisions against the shared privacy reference test corpus.

Introduces new androidTest fixtures (tests.json, config-reference.json, tds-reference.json, surrogates-reference.txt, user-allowlist-reference.json) and updates copy-files-from-to.json to sync these files from @duckduckgo/privacy-reference-tests.

Written by Cursor Bugbot for commit 89face1. This will update automatically on new commits. Configure here.

Copy link
Contributor Author

catalinradoiu commented Mar 9, 2026

@catalinradoiu catalinradoiu marked this pull request as ready for review March 9, 2026 16:20
@CrisBarreiro CrisBarreiro self-requested a review March 9, 2026 16:27
Base automatically changed from feature/cradoiu/add-exceptions-check-for-blocklist to develop March 10, 2026 10:37
…ce-tests

# Conflicts:
#	privacy-config/privacy-config-impl/src/main/java/com/duckduckgo/privacy/config/impl/features/requestblocklist/RealRequestBlocklist.kt
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't add these files manually to the repo, as reference tests are an external dependency that's versioned. Instead, we should bump the privacy-reference-tests dependency, and add the files to copy-from-to.json, so in case the contents of the files are updates, reference tests will always run on the latest version. See #5769 for reference

import java.util.concurrent.CopyOnWriteArrayList

@RunWith(Parameterized::class)
class RequestBlocklistReferenceTest(private val testCase: TestCase) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is testing WebViewRequestInterceptor, not RequestBlocklist, so I'd rename it

Copy link
Contributor Author

@catalinradoiu catalinradoiu Mar 12, 2026

Choose a reason for hiding this comment

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

I followed the same approach as DomainsReferenceTest. In that case we are testing the WebViewRequestInterceptor as well, but the test is actually a reference test. I wanted it to be clear that this test file is for request blockist reference tests and not a regular unit test for WebViewRequestInterceptor.

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Missing else branch silently passes unknown actions
    • Added an else branch in the expectAction switch that calls fail(...) so unknown external action values now fail the test explicitly.

Create PR

Or push these changes by commenting:

@cursor push 661262294e
Preview (661262294e)
diff --git a/app/src/androidTest/java/com/duckduckgo/app/referencetests/RequestBlocklistReferenceTest.kt b/app/src/androidTest/java/com/duckduckgo/app/referencetests/RequestBlocklistReferenceTest.kt
--- a/app/src/androidTest/java/com/duckduckgo/app/referencetests/RequestBlocklistReferenceTest.kt
+++ b/app/src/androidTest/java/com/duckduckgo/app/referencetests/RequestBlocklistReferenceTest.kt
@@ -87,6 +87,7 @@
 import org.json.JSONObject
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
+import org.junit.Assert.fail
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -247,6 +248,7 @@
         when (testCase.expectAction) {
             "block" -> assertCancelledResponse(response)
             "allow" -> assertRequestCanContinueToLoad(response)
+            else -> fail("Unsupported expectAction '${testCase.expectAction}' in request-blocklist reference test '${testCase.name}'")
         }
     }

when (testCase.expectAction) {
"block" -> assertCancelledResponse(response)
"allow" -> assertRequestCanContinueToLoad(response)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing else branch silently passes unknown actions

Low Severity

The when block handling testCase.expectAction only matches "block" and "allow" with no else branch. Since these are externally-versioned reference tests (from @duckduckgo/privacy-reference-tests), if the external package introduces a new action type (e.g., "redirect"), the test would complete without executing any assertion — silently passing as a no-op instead of failing. Adding an else branch that calls fail() with a descriptive message would prevent silent false-positive test results when the external test data evolves.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are using the same pattern with the other tests. In this case we want to check only for block and allow actions, as we don’t have redirect.

@catalinradoiu catalinradoiu merged commit 397f38a into develop Mar 12, 2026
12 checks passed
@catalinradoiu catalinradoiu deleted the feature/cradoiu/request-blocklist-reference-tests branch March 12, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants