DEV Community

sp0033212000
sp0033212000

Posted on

Team of One: Fixing a Broken Test Suite Between Feature Requests

In September I ran yarn test on our NestJS API and 71 of 87 suites failed.

Honestly, I wasn't surprised. I'm the only engineer at the company, so the website, the back office, the API and every deploy are on me. When a feature request comes in, the feature gets built first and the tests wait. And since CI never ran the tests, nobody knew they were broken.

But being the only engineer also means nobody reviews my code. Tests are the only safety net I've got. So I started fixing them in the gaps between feature work, a bit at a time. By the end of the month the suite was green, it ran in 95 seconds instead of 335, and a failing test blocked the deploy. This post is about how I decided what to fix first, and what not to fix at all, when there wasn't much time to go around.

How it got that bad

The API runs on Cloud Run and deploys through Cloud Build, which had two steps: build the image, deploy it. Tests never ran, so they quietly rotted.

Almost every failure looked like this, before a single assertion ran:

Nest can't resolve dependencies of the RentalService
(PrismaService, GoogleService, FormatHelperService, GoogleMapService, ?, UserService).
Please make sure that the argument SystemService at index [4] is available...
Enter fullscreen mode Exit fullscreen mode

The one that hurt was the rental service spec. It had 19 tests covering real business logic, and they used to pass. Then SystemService was added to the constructor and all 19 broke. With 70 other suites already red, nobody noticed. Me included.

Short on time, so save what matters

I didn't have time to fix 71 suites one by one, so the first step was triage: which of these were actually worth my time?

69 of them were Nest CLI scaffolding. nest g service generates a spec with a single it('should be defined') and provides the service without any of its dependencies, so any service with a constructor argument fails from day one. These had never passed.

That raised the obvious question: should I even keep them? My test was simple. If I delete this file, do we lose anything we know about how the code behaves? A should be defined stub only checks that Nest can wire the class, and the app already proves that every time it starts. Keeping them would mean a mock list per service, updated on every constructor change. For a team of one that cost is way too high, and the fact that it hadn't happened 69 times says it all.

So I fixed the 21 real tests first. The rental spec just needed mocks for its new dependencies. The listing controller spec was failing because AuthGuard needed a UserService. Instead of providing it, I overrode the guard:

const module = await Test.createTestingModule({
  controllers: [ListingController],
  providers: [{ provide: ListingService, useValue: listingServiceMock }],
})
  .overrideGuard(AuthGuard)
  .useValue({ canActivate: () => true })
  .compile();
Enter fullscreen mode Exit fullscreen mode

Guard behavior belongs in the auth module's own tests. Making every controller spec deal with the guard's dependencies is exactly what broke this one. With the override in place, future changes to the guard won't break controller tests, and I won't have to come back and fix them.

I kept the stubs for five core modules: listing, rental, users, webhook and auth. They handle data and permissions, so a broken constructor there hurts the most, and that's a maintenance cost I'm willing to pay. The other 62 went. I thought about describe.skip, but that just turns the screen green. The problem stays, and I'd have to deal with it all over again later.

Last, I ran the suite with the database URL and LINE credentials blanked out. Still green, so every test was properly mocked and CI wouldn't need any secrets.

Make a failing test block the deploy

Fixing the tests was only half the job. Nobody reviews my code, so without something forcing the issue, tests would start breaking again the next time I'm rushing a feature. Once the suite was green, I added a test step at the start of Cloud Build. Cloud Build stops at the first failing step, so a failing test means no image and no deploy.

Its first three runs on staging all failed, each for a different reason, and none of them reproduced on my laptop:

  • The Prisma client wasn't generated in the CI container, so every model type was missing. Fix: run prisma generate before the tests.
  • Jest ran out of memory while type-checking the generated client. Fix: raise Node's heap size and recycle workers with --workerIdleMemoryLimit.
  • One module created an SDK client from credentials at import time and crashed before any test ran. Fix: dummy values for those variables in CI.

Moving the tests to GitHub Actions

Running tests in Cloud Build worked, but every build reinstalled dependencies the Dockerfile had already cached, and failures were buried in the Cloud Build logs. Digging through those every time adds up when you're the only one doing it. So I moved the tests to GitHub Actions. The workflow only starts Cloud Build after the tests pass, and a failure shows up as a red X right on the commit.

The catch is that the build no longer protects itself. It's up to the workflow to make sure only tested commits get deployed. The relevant parts look like this:

concurrency:
  group: deploy-${{ github.ref }}
  cancel-in-progress: false

jobs:
  test:
    strategy:
      fail-fast: true
      matrix:
        shard: [1, 2, 3]
    steps:
      # checkout, setup-node, yarn install, prisma generate
      - run: yarn test --ci --shard=${{ matrix.shard }}/3

  deploy:
    needs: test
    permissions:
      id-token: write
    steps:
      # authenticate with Workload Identity Federation
      - run: gcloud builds triggers run "$TRIGGER" --sha="${{ github.sha }}" ...
Enter fullscreen mode Exit fullscreen mode
  • --sha builds the commit that passed. Without it, triggers run builds whatever the branch points to at that moment, so if I push again while the tests are running, the newer, untested commit gets deployed.
  • The per-branch concurrency group, with the deploy job waiting for the build to finish, means an older build can never land after a newer one and overwrite it.
  • Workload Identity Federation means there's no service account key stored in GitHub, which is one less secret to manage or worry about leaking. Only this repo's staging and main branches can get credentials.
  • GitHub's runner was noticeably slower than Cloud Build, and a single test job would have made deploys slower than before, so the suite runs as three parallel shards.

I also used triggers run rather than gcloud builds submit. submit uploads the working directory as the build source, and after a test run that directory is full of node_modules.

Checking that it works

I pushed a test I'd broken on purpose. One shard failed, the deploy job was skipped, and Cloud Build never saw that commit. That's when I actually trusted it.

The first timed run caught a mistake of mine too. Push to deploy took over 18 minutes, and it turned out I'd forgotten to remove the old test step from Cloud Build, so every commit was being tested twice. After removing it, it came down to about 12.

Where it ended up

Before After
Failing test suites 71 of 87 0 of 25
Test run 335 s 95 s
Tests block deploys No Yes
Where failures show Nowhere A red X on the commit
Push to production ~10 min, untested ~12 min, tested

Deploys take about as long as before, but every one is tested now, and the whole thing runs without me babysitting it. When you're the only engineer, that matters a lot more than saving a few minutes. If I were starting over, I'd add the gate on day one. Everything in this post is me catching up on what I didn't do back then.

Top comments (0)