Skip to content

add test#1341

Open
NancyWei123 wants to merge 2 commits into
AOSSIE-Org:mainfrom
NancyWei123:add-test
Open

add test#1341
NancyWei123 wants to merge 2 commits into
AOSSIE-Org:mainfrom
NancyWei123:add-test

Conversation

@NancyWei123

@NancyWei123 NancyWei123 commented Jun 25, 2026

Copy link
Copy Markdown

Addressed Issues:

Fixes #(TODO:issue number)
#1238

Screenshots/Recordings:

Before:
I forget to take sceenshot.
After:
image
image

TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.

Additional Notes:

AI Usage Disclosure:

We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.

Check one of the checkboxes below:

  • This PR does not contain AI-generated code at all.
  • [✓] This PR contains AI-generated code. I have read the AI Usage Policy and this PR complies with this policy. I have tested the code locally and I am responsible for it.

I have used the following AI models and tools: chatgpt

Checklist

  • [✓] My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • [✓] My code follows the project's code style and conventions
  • [✓] If applicable, I have made corresponding changes or additions to the documentation
  • [✓] If applicable, I have made corresponding changes or additions to tests
  • [✓] My changes generate no new warnings or errors
  • [✓] I have joined the Discord server and I will share a link to this PR with the project maintainers there
  • [✓] I have read the Contribution Guidelines
  • [✓] Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.
  • [✓] I have filled this PR template completely and carefully, and I understand that my PR may be closed without review otherwise.

Summary by CodeRabbit

  • Tests
    • Added database connection unit tests covering transaction commit/rollback behavior and required SQLite settings.
    • Expanded folder database unit tests for inserts, queries, updates, deletes, path-based lookups, and hierarchy operations.
    • Improved test isolation by switching to per-test temporary SQLite databases for more reliable, repeatable results.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 542a2c9b-f1e6-4269-b9ee-203f94ac472d

📥 Commits

Reviewing files that changed from the base of the PR and between 17ab49b and ad31a4f.

📒 Files selected for processing (2)
  • backend/tests/test_connection.py
  • backend/tests/test_folders.py
💤 Files with no reviewable changes (1)
  • backend/tests/test_connection.py

Walkthrough

Adds pytest coverage for SQLite connection handling and expands folders database tests with a temporary database fixture, schema reset helper, and unit cases for inserts, queries, deletes, subtree updates, AI-tagging updates, and folder-detail aggregation.

Changes

Database test coverage

Layer / File(s) Summary
SQLite connection behavior
backend/tests/test_connection.py
get_db_connection() is tested for commit-on-success, rollback-on-exception, and SQLite PRAGMA setup.
Folders fixture and unit coverage
backend/tests/test_folders.py
The folders test fixture now uses tmp_path and monkeypatch, and new unit tests cover folder inserts, lookups, deletes, subtree updates, AI-tagging updates, and aggregation queries.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • AOSSIE-Org/PictoPy#1285 — Both changes touch app.database.folders.db_get_all_folder_details and its image_count behavior.

Suggested labels

Python

Suggested reviewers

  • rohan-pandeyy

Poem

🐇 I hopped through SQLite, neat and new,
Commit or rollback, the tests all knew.
In folders deep, my burrow grew,
With temp paths bright and schemas true.
Hop hop—PRAGMA sparks a happy “boing!”

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is too vague to convey the main change; it only says tests were added. Use a concise, specific title that names the primary area changed, such as adding database connection and folders tests.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
backend/tests/test_connection.py (1)

61-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

case_sensitive_like is read but never asserted.

Line 61 reads PRAGMA case_sensitive_like into a variable that is never used. Either assert it (the upstream context manager sets it to ON, i.e. 1) or drop the read to keep the test focused.

♻️ Optional: assert the read pragma
 def test_get_db_connection_enables_pragmas(tmp_path):
     db_path = tmp_path / "test.db"

     with patch("app.database.connection.DATABASE_PATH", str(db_path)):
         with get_db_connection() as conn:
             foreign_keys = conn.execute("PRAGMA foreign_keys").fetchone()[0]
             ignore_check = conn.execute("PRAGMA ignore_check_constraints").fetchone()[0]
             recursive_triggers = conn.execute("PRAGMA recursive_triggers").fetchone()[0]
-            case_sensitive_like = conn.execute("PRAGMA case_sensitive_like").fetchone()
+            case_sensitive_like = conn.execute("PRAGMA case_sensitive_like").fetchone()[0]

     assert foreign_keys == 1
     assert ignore_check == 0
     assert recursive_triggers == 1
+    assert case_sensitive_like == 1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_connection.py` around lines 61 - 65, In
test_connection.py, the PRAGMA read for case_sensitive_like in the connection
test is unused, so either assert that the value is 1 like the other PRAGMA
checks or remove the read entirely to keep the test aligned with the context
manager behavior. Update the test around the existing foreign_keys,
ignore_check, and recursive_triggers assertions so the connection setup
verification remains consistent and focused.
backend/tests/test_folders.py (1)

1076-1079: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix the misplaced section-header indentation.

Line 1076 is indented as if it belongs to TestFoldersUnit's body while lines 1077-1078 are dedented, leaving a ragged comment block before TestFoldersIntegration. Realign the header and add separation for readability.

♻️ Optional: align header
-        # ============================================================================
-    # Integration & Workflow Tests
-    # ============================================================================
+
+
+# ============================================================================
+# Integration & Workflow Tests
+# ============================================================================
 class TestFoldersIntegration:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_folders.py` around lines 1076 - 1079, The section header
before TestFoldersIntegration is misindented and visually detached from the
class it introduces. Realign the comment block so the separator lines and
“Integration & Workflow Tests” header use consistent indentation, and add a
blank line before the TestFoldersIntegration class definition to make the
boundary clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/tests/test_connection.py`:
- Around line 49-51: Remove the redundant second assert in the test and replace
the misleading rollback comment with one that matches the actual behavior. In
the test around the `result` checks, keep only one `assert result is None` and
update the comment near the `SELECT` assertion to state that the `Bob` insert
was rolled back while the earlier `CREATE TABLE` in the first `with` block was
committed.

---

Nitpick comments:
In `@backend/tests/test_connection.py`:
- Around line 61-65: In test_connection.py, the PRAGMA read for
case_sensitive_like in the connection test is unused, so either assert that the
value is 1 like the other PRAGMA checks or remove the read entirely to keep the
test aligned with the context manager behavior. Update the test around the
existing foreign_keys, ignore_check, and recursive_triggers assertions so the
connection setup verification remains consistent and focused.

In `@backend/tests/test_folders.py`:
- Around line 1076-1079: The section header before TestFoldersIntegration is
misindented and visually detached from the class it introduces. Realign the
comment block so the separator lines and “Integration & Workflow Tests” header
use consistent indentation, and add a blank line before the
TestFoldersIntegration class definition to make the boundary clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c4b640b2-d2af-4ae7-ad8b-06273d3778a4

📥 Commits

Reviewing files that changed from the base of the PR and between b3eee2c and 17ab49b.

📒 Files selected for processing (2)
  • backend/tests/test_connection.py
  • backend/tests/test_folders.py

Comment thread backend/tests/test_connection.py Outdated
@NancyWei123

Copy link
Copy Markdown
Author
image add more coverage for folders.py
fix problem

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant