Skip to content

fix: add rule to prevent test files from having the same name#9671

Merged
dmadisetti merged 1 commit into
mainfrom
dm/pycache-bust
May 22, 2026
Merged

fix: add rule to prevent test files from having the same name#9671
dmadisetti merged 1 commit into
mainfrom
dm/pycache-bust

Conversation

@dmadisetti
Copy link
Copy Markdown
Collaborator

📝 Summary

A common flaky failure pattern is:

ERROR tests/_messaging/test_variables.py - import file mismatch:
imported module 'test_variables' has this __file__ attribute:
  /home/runner/work/marimo/marimo/tests/_ast/test_variables.py
which is not the same as the test file we want to collect:
  /home/runner/work/marimo/marimo/tests/_messaging/test_variables.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules

Which occurs because we have test files named the same thing (e.g. test_variables.py here).

This PR moves some of these existing files, and adds a pre commit rule to stop users duplicating additional tests.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 22, 2026 5:46pm

Request Review

@dmadisetti dmadisetti changed the title fix: add rule to prevent test files being called the same fix: add rule to prevent test files from having the same name May 22, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 6 files

Architecture diagram
sequenceDiagram
    participant Dev as Developer
    participant Git as Git Hook (pre-commit)
    participant Script as check_test_filenames.py
    participant Fs as tests/ Directory
    participant Ignore as Ignored Paths

    Note over Dev,Ignore: Pre-commit Hook Flow

    Dev->>Git: git commit
    Git->>Script: python scripts/check_test_filenames.py
    Script->>Fs: Walk tests/ for test_*.py files
    Fs-->>Script: List of file paths

    Script->>Script: Check if path is ignored
    alt Path in IGNORED_PREFIXES
        Script->>Ignore: Skip path
    end

    Script->>Script: Group by filename (e.g., test_variables.py)
    Script->>Script: Check for duplicate basenames

    alt Duplicate basenames found
        Script-->>Git: Non-zero exit (1)
        Git-->>Dev: Reject commit, print duplicate names
    else No duplicates
        Script-->>Git: Zero exit (0)
        Git-->>Dev: Allow commit to proceed
    end

    Note over Dev,Ignore: Pytest Collection Context

    Dev->>Fs: Run pytest
    Fs->>Fs: Collect test files
    alt Duplicate basenames exist (blocked by hook)
        Fs->>Fs: Import clobbering occurs
        Fs-->>Dev: Import file mismatch error
    else Unique basenames (enforced by hook)
        Fs-->>Dev: Successful test collection
    end
Loading

Re-trigger cubic

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

This PR addresses a recurring pytest collection flake caused by duplicate test module basenames (pytest’s default import mode imports test files as top-level modules). It introduces a pre-commit check to enforce unique test filenames and adds/relocates several tests under new, non-colliding filenames.

Changes:

  • Added a repository script to detect duplicate test basenames under tests/ and fail fast.
  • Wired the duplicate-filename check into pre-commit via a local hook.
  • Added/relocated multiple test modules (session/notebook, session/managers, code_mode) under unique basenames.

Reviewed changes

Copilot reviewed 2 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/_session/notebook/test_filesystem_storage.py Adds FilesystemStorage coverage; includes a couple tests intending to validate string-path handling.
tests/_session/notebook/test_app_file_manager.py Adds tests for load_notebook / new_notebook helpers and path resolution behavior.
tests/_session/managers/test_app_host_kernel_manager.py Adds unit tests for AppHost kernel/queue manager adapters.
tests/_code_mode/test_code_mode_context.py Adds/relocates comprehensive tests for AsyncCodeModeContext transaction behavior.
scripts/check_test_filenames.py New enforcement script to prevent duplicate test basenames (core of the flake fix).
.pre-commit-config.yaml Adds a local pre-commit hook to run the new test-filename check.

Comment thread scripts/check_test_filenames.py
@dmadisetti dmadisetti merged commit 3dd6955 into main May 22, 2026
43 checks passed
@dmadisetti dmadisetti deleted the dm/pycache-bust branch May 22, 2026 18:19
@github-actions
Copy link
Copy Markdown

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.9-dev2

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.

3 participants