Remove fakefs gem from bosh-director#2767
Open
aramprice wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
💤 Files with no reviewable changes (4)
WalkthroughRemoved FakeFS from development and test dependencies. Updated shared helpers to stub configuration reads, and refactored affected specs to use temporary real filesystem paths for task outputs, rendered templates, and CPI logs. Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
aramprice
requested review from
a team,
ay901246 and
ramonskie
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the fakefs dependency from bosh-director by migrating unit specs from FakeFS-based isolation to real temporary directories (Dir.mktmpdir) and targeted File stubs.
Changes:
- Dropped
fakefsfrom development/test dependencies (Gemfile, gemspec, lockfile) and removed the vendored gem. - Updated specs to use real tmpdirs for filesystem interactions (CPI task logs, rendered templates output, task outputs, etc.).
- Replaced FakeFS file cloning/creation with
File.readstubs for NATS certificate paths used by the test director config fixture.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vendor/cache/fakefs-3.2.1.gem | Removes the vendored FakeFS gem artifact. |
| src/Gemfile | Drops fakefs from the development/test group. |
| src/Gemfile.lock | Removes fakefs from locked dependencies. |
| src/bosh-director/bosh-director.gemspec | Drops fakefs as a development dependency. |
| src/bosh-director/spec/support/file_helpers.rb | Replaces FakeFS cloning/file creation with File.read stubbing for cert paths. |
| src/bosh-director/spec/unit/clouds/external_cpi_spec.rb | Uses tmpdir-backed log path instead of FakeFS /var/vcap/.... |
| src/bosh-director/spec/unit/clouds/external_cpi_response_wrapper_spec.rb | Uses tmpdir-backed log path instead of FakeFS /var/vcap/.... |
| src/bosh-director/spec/unit/bosh/director/jobs/ssh_spec.rb | Removes FakeFS usage from SSH job specs. |
| src/bosh-director/spec/unit/bosh/director/deployment_plan/instance_spec.rb | Removes FakeFS helpers; relies on stubbed config file reads. |
| src/bosh-director/spec/unit/bosh/director/core/templates/rendered_templates_writer_spec.rb | Writes rendered templates into a real tmpdir instead of /out. |
| src/bosh-director/spec/unit/bosh/director/core/templates/compressed_rendered_job_templates_spec.rb | Uses tmpdir for #contents/#sha1; adjusts #write expectation without FakeFS. |
| src/bosh-director/spec/unit/bosh/director/config_spec.rb | Removes FakeFS helpers; relies on stubbed config file reads. |
| src/bosh-director/spec/unit/bosh/director/api/task_remover_spec.rb | Uses tmpdir-backed task output dirs and removes FakeFS activation toggling. |
| src/bosh-director/spec/unit/bosh/director/api/snapshot_manager_spec.rb | Removes FakeFS helpers; relies on stubbed config file reads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aramprice
force-pushed
the
remove-fake-fs
branch
2 times, most recently
from
1049214 to
a473d2e
Compare
aramprice
force-pushed
the
remove-fake-fs
branch
from
a473d2e to
2851866
Compare
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/bosh-director/spec/support/file_helpers.rb`:
- Around line 24-30: Rename configure_fake_config_files to a name reflecting its
behavior, such as stub_nats_config_file_reads, and update every call site
accordingly; keep the existing File.read stubs unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 636540da-c157-4955-9ec1-ffd809ccf43b
⛔ Files ignored due to path filters (1)
src/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
src/Gemfilesrc/bosh-director/bosh-director.gemspecsrc/bosh-director/spec/support/file_helpers.rbsrc/bosh-director/spec/unit/bosh/director/api/snapshot_manager_spec.rbsrc/bosh-director/spec/unit/bosh/director/api/task_remover_spec.rbsrc/bosh-director/spec/unit/bosh/director/config_spec.rbsrc/bosh-director/spec/unit/bosh/director/core/templates/compressed_rendered_job_templates_spec.rbsrc/bosh-director/spec/unit/bosh/director/core/templates/rendered_templates_writer_spec.rbsrc/bosh-director/spec/unit/bosh/director/deployment_plan/instance_spec.rbsrc/bosh-director/spec/unit/bosh/director/jobs/ssh_spec.rbsrc/bosh-director/spec/unit/clouds/external_cpi_response_wrapper_spec.rbsrc/bosh-director/spec/unit/clouds/external_cpi_spec.rbsrc/vendor/cache/fakefs-3.2.1.gem
💤 Files with no reviewable changes (6)
- src/bosh-director/spec/unit/bosh/director/jobs/ssh_spec.rb
- src/Gemfile
- src/bosh-director/bosh-director.gemspec
- src/vendor/cache/fakefs-3.2.1.gem
- src/bosh-director/spec/unit/bosh/director/api/snapshot_manager_spec.rb
- src/bosh-director/spec/unit/bosh/director/deployment_plan/instance_spec.rb
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
remove-fake-fs
branch
from
2851866 to
95dfbec
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/bosh-director/spec/unit/bosh/director/core/templates/compressed_rendered_job_templates_spec.rb:50
subject.contentsreturns an open file handle (File.open). This example reads from it but never closes it, which can leak file descriptors across the test suite (more noticeable now that the real filesystem is used). Close the IO in anensureblock after reading.
it 'returns IO object for given path' do
expect(subject.contents.readlines).to eq(['fake-content'])
end
95dfbec to
a700968
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/bosh-director/spec/unit/bosh/director/config_spec.rb`:
- Around line 29-33: Remove the nested after hook near the identity_provider
examples that calls FileUtils.rm_rf(temp_dir), since the top-level cleanup hook
already covers all tests in the Bosh::Director::Config spec. Retain the
top-level after hook and ensure cleanup behavior remains unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92d6e34f-a0c1-4e94-aa86-cc28147b3201
⛔ Files ignored due to path filters (1)
src/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
src/Gemfilesrc/bosh-director/bosh-director.gemspecsrc/bosh-director/spec/support/file_helpers.rbsrc/bosh-director/spec/unit/bosh/director/api/snapshot_manager_spec.rbsrc/bosh-director/spec/unit/bosh/director/api/task_remover_spec.rbsrc/bosh-director/spec/unit/bosh/director/config_spec.rbsrc/bosh-director/spec/unit/bosh/director/core/templates/compressed_rendered_job_templates_spec.rbsrc/bosh-director/spec/unit/bosh/director/core/templates/rendered_templates_writer_spec.rbsrc/bosh-director/spec/unit/bosh/director/deployment_plan/instance_spec.rbsrc/bosh-director/spec/unit/bosh/director/jobs/ssh_spec.rbsrc/bosh-director/spec/unit/clouds/external_cpi_response_wrapper_spec.rbsrc/bosh-director/spec/unit/clouds/external_cpi_spec.rbsrc/vendor/cache/fakefs-3.2.1.gem
💤 Files with no reviewable changes (4)
- src/vendor/cache/fakefs-3.2.1.gem
- src/Gemfile
- src/bosh-director/spec/unit/bosh/director/jobs/ssh_spec.rb
- src/bosh-director/bosh-director.gemspec
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
Replace all FakeFS usage in unit specs with Dir.mktmpdir-based real filesystem isolation and allow(File).to receive stubs, so the gem can be dropped entirely. Changes per file: - spec/support/file_helpers.rb: replace FakeFS::FileSystem.clone and fake-file creation with allow(File).to receive(:read) stubs for cert paths read from the YAML fixture (so stubs stay aligned if fixture changes) - external_cpi_spec.rb, external_cpi_response_wrapper_spec.rb: use a real tmpdir for the CPI task log directory; remove FileUtils.mkdir_p on the fake /var/vcap path - config_spec.rb, instance_spec.rb, snapshot_manager_spec.rb: remove FakeFS::SpecHelpers (configure_fake_config_files now uses stubs) - rendered_templates_writer_spec.rb: write rendered templates into a real Dir.mktmpdir instead of the hardcoded /out path - compressed_rendered_job_templates_spec.rb: scope Dir.mktmpdir-backed tmpdir to the #contents and #sha1 contexts; #write uses a static path since all filesystem operations are mocked there - task_remover_spec.rb: create task output dirs under a real tmpdir; update Dir[] assertions accordingly; use File.exist? instead of Dir[...].to_not be_nil (Dir[] returns Array, never nil); remove FakeFS.deactivate!/activate! - ssh_spec.rb: remove FakeFS entirely (Jobs::Ssh performs no filesystem I/O - it writes results via TaskDBWriter to the DB) - Gemfile, bosh-director.gemspec: drop fakefs dependency - Gemfile.lock: updated by bundle install
aramprice
force-pushed
the
remove-fake-fs
branch
from
a700968 to
923ea79
Compare
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
Alphasite
approved these changes
Jul 11, 2026
Alphasite
left a comment
Alphasite
left a comment
Contributor
There was a problem hiding this comment.
left a couple comments but it seems ok...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace all FakeFS usage in unit specs with Dir.mktmpdir-based real filesystem isolation and allow(File).to receive stubs, so the gem can be dropped entirely.
Changes per file: