denoland / saaskit

A modern SaaS template built on Fresh.
https://deno.com/saaskit
MIT License
1.21k stars 148 forks source link

chore: improve atomicity of E2E tests #650

Closed azohra closed 10 months ago

azohra commented 10 months ago

Issue

The e2e test runner was setting ENV from multiple sources leading to cognitive complexity when reading, writing, and running tests. In particular, Github vars were passed as part of deno task test while STRIPE entities were not.

Observations

  1. Tests had different outcomes if run outside of the deno.json test task.
  2. Tests were not atomically runnable as they were unaware of their required ENV vars.
  3. A users ENV could alter the outcome of tests

Improvement

I created a setupEnv function which defines the base ENV state our application expects and provides a mechanism for mutating it into what a given test requires.

  1. All tests start with the same base

  2. Tests are atomic
  3. Deno task args minimally impact test outcomes (👀 DENO_KV_PATH)
  4. User ENV minimally impacts test outcomes

Example Behaviours

Pass a computed value as the input for an Environment Variable

    const priceId = crypto.randomUUID();
    setupEnv(
      { "STRIPE_PREMIUM_PLAN_PRICE_ID": priceId },
    );

Ensure an Environment Variable is unset

    setupEnv(
      { "STRIPE_SECRET_KEY": null },
    );
azohra commented 10 months ago

As a newbie to Deno, I am unclear if a fresh isolate is created for each test. If so, only part of this change is needed in which:

  1. we setup the env once at the start of the file
  2. we only call setupEnv inside a test for delete/modify events, or this would be where a direct call to Deno.env.delete makes sense.

The key item I wanted to address was the assumption of GITHUB_CLIENT_ID, and GITHUB_CLIENT_SECRET being set as a result of polluting the env in the deno task test command. Anything past that, I am a bit out of my depth as it pertains to Deno test runner quirks.