[CSM-230]: Added first unit tests for component

[boztopuz]

No description provided.

[cagkanaksunyuksel]

Code Review — PR #3: Add Unit Tests for Firebase Upload dSYM Component

Verified locally: 32/32 tests passing. Coverage: 72.2% (39/54 lines).

---

🔴 Must Fix

1. No CI — GitHub Actions workflow is missing

The acceptance criteria explicitly requires tests to run on every PR. There is no .github/workflows/ directory in this branch. Suggested minimum:

# .github/workflows/test.yml
name: Tests
on: [push, pull_request]
jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: ruby/setup-ruby@v1
        with:
          ruby-version: '3.2'
      - run: gem install rspec plist
      - run: ruby test/test_main.rb

---

🟡 Should Fix

2. run_command failure test does not assert the error message

it 'calls abort (SystemExit) when the command exits non-zero' do
  expect { run_command('false') }.to raise_error(SystemExit)
end

This confirms that abort is called, but not that it carries the stderr output. The real risk being tested is that failure details are surfaced to the caller. Consider:

it 'aborts with stderr output when the command exits non-zero' do
  expect { run_command('bash -c "echo failure details >&2; exit 1"') }
    .to raise_error(SystemExit)
  # alternatively: capture $stderr and assert it includes 'failure details'
end

3. run_command stdout noise during test run

The lines @@[command] echo hi / hi / @@[command] false are printed to stdout during the test run because run_command calls puts unconditionally. Consider suppressing output in the test context:

allow($stdout).to receive(:puts)

Or better, refactor run_command to accept an output stream parameter. Not a blocker, but worth addressing since this is the reference implementation for all future component tests.

4. "Required libraries" test group adds low value

it 'makes Open3 available after requiring main.rb' do
  out, _err, status = Open3.capture3("ruby -e \"require '#{MAIN_RB}'; puts Open3.name\"")
  ...
end

These 3 tests each spawn a subprocess to verify that Open3, FileUtils, and Plist are accessible after require. Their availability depends on Ruby installation and require statements in main.rb, not on any logic we've written. If someone removes require 'open3' from main.rb, the #run_command tests will break first and more clearly. These tests add ~1.6s to the run time and don't guard against any real regression. Recommend removing them.

---

📋 Coverage Gap (informational)

Lines 58–73 are uncovered — these are the top-level ENV validation and upload execution lines inside if __FILE__ == $PROGRAM_NAME. The ENV validation describe block covers this path via subprocess (run_main), but the Coverage module only tracks in-process execution. This is expected and acceptable given the current architecture. No action needed.

---

✅ What's Done Well

• get_env_variable tests cover all 3 scenarios cleanly with proper around cleanup.
• find_dsyms test suite is thorough: no dSYM, all=true, all=false, missing plist, malformed plist, missing ApplicationProperties — all covered.
• allow(self).to receive(:run_command) correctly prevents real shell execution in find_dsyms tests.
• ENV validation block using run_main subprocesses is a solid integration layer and covers AC_REPOSITORY_DIR validation that wasn't even in the original issue scope.
• build_xcarchive helper is clean and reusable — good pattern for future component tests.
• Custom ReadableFormatter output is clear and well-structured.
• README accurately reflects how to run the tests.

---

Summary: 1 hard blocker (CI), 3 improvements worth addressing. The test logic itself is solid — just needs the workflow file before merge.