Revert "fix: handle glob patterns when expanded by the shell"

[rhamzeh]

Reverts #583 for the reasons described here: #583 (comment)

We might re-apply it, but we need some time to ensure we're doing it properly, so it has to go out of this release

Summary by CodeRabbit

• Refactor
  • Streamlined the test command to accept a single tests pattern (with glob support) while still handling multiple files via expansion.
  • Simplified argument handling and improved validation when locating test files.
• Bug Fixes
  • More reliable resolution of test files: falls back to a literal path when glob matches none, with clearer error messages.
• Chores
  • Removed an outdated example test configuration file.
  • Adjusted ignore rules to include .DS_Store files in version control.
  • Cleaned up internal tests related to deprecated test file pattern handling.

[coderabbitai]

Walkthrough

Refactors cmd/model test command to accept a single --tests string and use filepath.Glob with a fallback to literal path existence. Removes expandTestFilePatterns and its tests. Deletes an example YAML test file. Updates .gitignore by removing .DS_Store entry.

Changes

Cohort / File(s)	Summary of Changes
CLI test command refactor cmd/model/test.go	Replaced StringArray --tests with single String flag; switched to filepath.Glob; added fallback to literal path if no glob matches; removed expandTestFilePatterns and related error handling; simplified RunE args handling.
Unit tests removal cmd/model/test_test.go	Deleted entire test suite covering expandTestFilePatterns and related scenarios.
Examples cleanup example/subdir/simple.fga.yaml	Deleted example FGA model, tuples, and test case YAML.
Repo config .gitignore	Removed .DS_Store ignore entry.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant C as Cobra Command
  participant R as RunE (test)
  participant G as filepath.Glob
  participant FS as FileSystem

  U->>C: openfga model test --tests "<pattern or path>"
  C->>R: Execute
  R->>G: Glob(tests)
  alt Matches found
    G-->>R: []files
    R->>R: Use matched files
  else No matches
    R->>FS: Stat(literal path)
    alt Exists
      FS-->>R: OK
      R->>R: Use single file
    else Not found
      FS-->>R: NotFound
      R->>R: Return error: no files matched or path missing
    end
  end
  R-->>C: Result (files or error)
  C-->>U: Output summary/error

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: handle glob patterns when expanded by the shell #583 — Touches the same cmd/model/test.go area for --tests handling and glob expansion, but with an opposing approach (adds expandTestFilePatterns and StringArray), indicating a direct conceptual conflict.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly and concisely states that this pull request reverts the previous fix for handling shell‐expanded glob patterns, which matches the PR’s stated objective of undoing that change and does not include extraneous details or vague terms.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch revert-583-fix/glob-patterns

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef26542 and 74759a6.

📒 Files selected for processing (4)

• .gitignore (0 hunks)
• cmd/model/test.go (3 hunks)
• cmd/model/test_test.go (0 hunks)
• example/subdir/simple.fga.yaml (0 hunks)

💤 Files with no reviewable changes (3)

• .gitignore
• example/subdir/simple.fga.yaml
• cmd/model/test_test.go

🔇 Additional comments (4)

cmd/model/test.go (4)

39-39: LGTM: Appropriate use of blank identifier.

The positional args are unused, so the blank identifier is the idiomatic choice.

---

56-66: Glob expansion logic is sound.

The implementation correctly:

1. Uses filepath.Glob for pattern expansion
2. Falls back to literal path existence check when glob returns no matches
3. Provides clear error messages for both cases

Note: If a user has a literal filename containing glob metacharacters (e.g., test[1].yaml), they would need to escape them or the glob will fail to match. This is standard glob behavior but could be documented if it becomes a common issue.

---

145-145: LGTM: Flag definition updated consistently.

The flag definition correctly uses String instead of StringArray and the description appropriately mentions "Path or glob" to indicate the new single-pattern behavior.

---

41-44: No documentation update needed for --tests flag
Documentation already describes --tests as accepting a single string or glob pattern (e.g. "tests/*.fga.yaml"), and examples in README.md and docs/STORE_FILE.md correctly use that syntax.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}