Skip to content

IDE-31: Add unit tests for suspicious brick security warning#5176

Open
harshsomankar123-tech wants to merge 4 commits intoCatrobat:developfrom
harshsomankar123-tech:feature/IDE-31-missing-tests-suspicious-bricks
Open

IDE-31: Add unit tests for suspicious brick security warning#5176
harshsomankar123-tech wants to merge 4 commits intoCatrobat:developfrom
harshsomankar123-tech:feature/IDE-31-missing-tests-suspicious-bricks

Conversation

@harshsomankar123-tech
Copy link
Copy Markdown
Member

@harshsomankar123-tech harshsomankar123-tech commented Mar 29, 2026

Description

JIRA: https://catrobat.atlassian.net/browse/IDE-31
This PR introduces comprehensive automated test coverage for the "suspicious bricks" security warning feature (CATROID-681), which alerts users when a downloaded project contains potentially dangerous combinations of voice-listening (StartListeningBrick) and web-access bricks (WebRequestBrick, BackgroundRequestBrick, or LookRequestBrick).

Context & Problem: The existing detection logic in ProjectUtils.kt (getListAllBricks()) relies on a manual when block to traverse composite bricks. This architectural fragility previously caused regressions where suspicious bricks nested inside control structures went undetected, compromising the privacy warning.

Solution & Implementation:
Added 36 unit tests following standard JUnit/Kotlin assertion patterns to prevent future regressions:

  • GetListAllBricksTest.kt (13 tests): Verifies that getListAllBricks() correctly flattens and traverses all 9 composite brick types (e.g., ForeverBrick, IfLogicBeginBrick, RepeatBrick), including deep nesting scenarios like loops within if-statements.
  • SuspiciousBricksDetectionTest.kt (23 tests): * Unit Level: Verifies all suspicious combinations and ensures no false positives occur (e.g., voice listening without web access).
    • Integration Level: Validates detection when bricks are spread across different scripts, sprites, or scenes.
    • Regression Focus: Specifically tests scenarios where suspicious bricks are hidden inside nested loops to prevent historical bugs from reoccurring.

Note: The tests utilize Java reflection to verify private core detection functions (containsSuspiciousBricks and shouldDisplaySuspiciousBricksWarning) without compromising production encapsulation. StaticSingletonInitializer is used for correct environment setup.

Checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

  • Include the name of the Jira ticket in the PR’s title
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing unit tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Stick to the project’s gitflow workflow
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Post a message in the catroid-stage or catroid-ide Slack channel and ask for a code reviewer

@harshsomankar123-tech
Copy link
Copy Markdown
Member Author

@wslany could you please review this when you have time?
Thank you!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds new unit tests to prevent regressions in “suspicious bricks” security warning detection (CATROID-681 / IDE-31), focusing on Sprite.getListAllBricks() traversal and the private suspicious-brick detection helpers in ProjectUtils.kt.

Changes:

  • Adds GetListAllBricksTest coverage for flattening across the currently-handled composite brick types.
  • Adds SuspiciousBricksDetectionTest coverage for suspicious combinations and project-wide aggregation across scripts/sprites/scenes (via reflection on private helpers).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
catroid/src/test/java/org/catrobat/catroid/test/ui/SuspiciousBricksDetectionTest.kt Adds unit/integration-style tests for suspicious-bricks detection and project-level warning decision (via reflection).
catroid/src/test/java/org/catrobat/catroid/test/ui/GetListAllBricksTest.kt Adds tests validating getListAllBricks() behavior across supported composite bricks and some nesting scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@harshsomankar123-tech harshsomankar123-tech added the Active Member Tickets that are assigned to members that are still currently active label Apr 6, 2026
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Active Member Tickets that are assigned to members that are still currently active

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants