Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 24, 2025

Add specs for cache store with non-git-ignored directory

This PR adds specs that:

  • Create a cache store against a directory that is NOT git-ignored (spec/fixtures/committed_cache)
  • Populate the directory with cache files on first run (3 cache entries = 6 files)
  • Verify that no new files are created in subsequent runs
  • Check in the cache files so future PRs will pass

The tests ensure cache stability by:

  1. Writing predefined cache entries as JSON (user profiles and app settings)
  2. Verifying that reading existing entries doesn't create new files
  3. Verifying that writing same values to existing keys doesn't create new files
  4. Verifying that multiple read operations don't create new files
  5. Validating file count remains at 7 (6 cache files + 1 README.md)

All 51 tests pass (36 original + 15 new).

Changes in latest commit:

  • Extracted repeated Dir.glob calls into top-level let statements
  • Added file list helpers: key_files, value_files, all_files, key_contents
  • Simplified shared examples and test assertions to use the helpers
  • Improved code readability and DRY principles

Plan:

  • Create a new spec file for non-git-ignored cache directory tests
  • Create a non-git-ignored cache directory (spec/fixtures/committed_cache)
  • Write tests that populate the cache on first run
  • Write tests that verify no new files are created
  • Run tests to populate the cache files
  • Commit the cache files to source control
  • Verify tests pass in subsequent runs (after commit)
  • Address initial code review feedback and refactor
  • Address PR review comments: use shared examples, raw mode, readable JSON
  • DRY up code by extracting Dir.glob calls to let statements
Original prompt

Add specs that where there is one that makes a cache store against a directory that is not git-ignored and therefore stores a few files. Run it once to populate the files, then have tests that start making sure that no new files are created. Thus when we check in the test and those files, future PRs will pass as those files won't change / new ones won't get added.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Nov 24, 2025
Copilot AI and others added 2 commits November 24, 2025 18:57
Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com>
Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com>
Copilot AI changed the title [WIP] Add specs for cache store with directory verification Add specs for committed cache store to prevent file creep Nov 24, 2025
Copilot AI requested a review from nhorton November 24, 2025 19:04
Copilot finished work on behalf of nhorton November 24, 2025 19:04
Copy link
Contributor

@nhorton nhorton left a comment

Choose a reason for hiding this comment

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

RSpec.describe "Committed Cache Directory" do
# Use a non-git-ignored directory for cache storage
let(:committed_cache_path) { File.join(__dir__, "fixtures", "committed_cache") }
let(:store) { ActiveSupport::Cache::SourceControlCacheStore.new(cache_path: committed_cache_path) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Set this to not compress, and either set raw: true here if you can, or in all the read & write calls elsewhere so that the value files are in text.

Also remove the bad value files since the raw versions would be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configured in commit 0eec3e5:

  • Store initialized with compress: false
  • All read/write calls use raw: true
  • Cache entries now stored as JSON strings for readability
  • Regenerated all value files - they now contain readable JSON like {"name":"John Doe","email":"john@example.com"}

- Remove initial cache population block (files now exist)
- Add shared examples for cache validation
- Move cache entries to top-level let blocks
- Configure store with compress: false and raw: true
- Use JSON for readable value files
- Regenerate value files with readable JSON format

Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com>
Copilot AI requested a review from nhorton November 24, 2025 19:21
Copilot finished work on behalf of nhorton November 24, 2025 19:21
Copy link
Contributor

@nhorton nhorton left a comment

Choose a reason for hiding this comment

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

@copilot - almost there.

Just clean up that you have lots of examples where you set a variable to equal Dir.glob.... Most of those could be let statements, many at a high-level in the file. Ex. let(:value_files) and such. Make that refactor to DRY this up.

- Extract Dir.glob calls to top-level let statements
- Add key_files, value_files, all_files, and key_contents helpers
- Simplify shared examples to use let helpers
- Simplify test assertions by reusing all_files instead of inline Dir.glob
- Reduces code duplication and improves readability

Co-authored-by: nhorton <204146+nhorton@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 24, 2025

@copilot - almost there.

Just clean up that you have lots of examples where you set a variable to equal Dir.glob.... Most of those could be let statements, many at a high-level in the file. Ex....

Refactored in commit f4e2f2c:

  • Extracted all Dir.glob calls to top-level let statements
  • Added helpers: key_files, value_files, all_files, key_contents
  • Simplified shared examples and test assertions to use these helpers
  • Reduced code duplication from 147 to 130 lines

Copilot AI requested a review from nhorton November 24, 2025 20:58
Copilot finished work on behalf of nhorton November 24, 2025 20:58
@nhorton nhorton marked this pull request as ready for review November 26, 2025 15:06
@nhorton nhorton merged commit ee6cf71 into main Nov 26, 2025
4 checks passed
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