我在编写测试时犯的一系列不幸的错误

很久以前（其实没那么久，大概几年前），我开始写测试的时候，还是个天真的年轻人。我讨厌 Bug，所以我写测试，而且我当时的知识有限，只能靠自己有限的知识来写。

天真和不完全了解参考文献是要付出代价的。从每一次被拒绝的 PR 评审或回归 bug 中，我都从错误中吸取了教训，也意识到自己还有很多需要改进的地方。对我来说，通过反复尝试来学习确实很不幸，但对你来说，这不一定是不幸！

各位开发者，如果您觉得您的测试不够好，或者您的 PR 因为测试质量不佳而被 QA 团队拒绝了太多次，那么这篇文章或许对您有所帮助。我将与您分享我在编写测试时犯过的五个最严重的错误，以及为什么您应该避免这些错误。

在此之前，先声明一下：以下示例代码是用 JavaScript 编写的，并使用 Jest 作为测试框架。我主要关注 JavaScript，所以无法对其他语言进行过多评论，因为我不确定它是否适用。另外，这些只是简化的示例，并不代表实际用例。只是为了说明一下。

好的。接下来看例子。假设我正在写这个类：

class Account {
  constructor (initialBalance = 0) {
    this.balance = initialBalance
  }

  deposit (money) {
    this.balance += money
  }

  withdraw (money) {
    this.balance -= money
  }
}

目前，这个类还比较简单。它提供了一种存取金额的方法，可以改变余额。我的测试编写之旅就从这里开始。

1. 测试不够简单

我首先想测试的是.deposit方法。在我看来，测试必须非常具体，其他读到测试的人甚至不需要看实际代码。

const account = new Account()

describe('Account class', () => {
  describe('.deposit', () => {
    test('Should increment the account balance by the amount', () => {
      const increment = 200
      const originalBalance = account.balance
      account.deposit(increment)
      expect(account.balance).toBe(originalBalance + increment)
    })
  })
})

测试看起来不错，对吧？它有原始余额、需要增加的金额，并且断言了原始余额加上增量。事实上，如果我想更改增量的金额，只需要更改变量increment，测试仍然会通过。就是这样。超级简单。

然后又有一个新的要求，每存入一笔钱，都会加2%，作为奖励（别问我为什么，这是总理……）。

  deposit (money) {
    this.balance += (money * 1.02)
  }

嗯，是的，好的。那么测试应该是……

    test('Should increment the account balance by the amount plus 2% incentive', () => {
      const increment = 200
      const originalBalance = account.balance
      // PLEASE SEE TEH CODE FOR THE CLASS FOR REFERENCE
      const incrementPlusIncentive = increment * 1.02
      account.deposit(increment)
      expect(account.balance).toBe(originalBalance + incrementPlusIncentive)
    })

哦，天哪，这到底是什么怪物？我本来想把它弄清楚，结果却弄得更复杂了。而且，我还把代码里的逻辑复制到测试里了。这不对劲。

实践中，测试代码应该只明确说明测试内容（输入->输出）。不应该包含任何逻辑代码；逻辑代码本身就是测试代码的一部分。因此，改进后的版本应该是：

    test('Should increment the account balance by the amount plus 2% incentive', () => {
      account.deposit(100)
      expect(account.balance).toBe(102)
    })

好了。简单点。我存了100，我的余额现在是102。符合要求吗？完全符合！这才是最重要的。

2. 每次测试时没有保持清洁状态

我的下一个任务是完成剩下的测试。.withdraw是的。

const account = new Account()

describe('Account class', () => {
  describe('.deposit', () => {
    test('Should increment the account balance by the amount plus 2% incentive', () => {
      account.deposit(100)
      expect(account.balance).toBe(102)
    })
  })

  describe('.withdraw', () => {
    test('Should decrement the account balance by the amount', () => {
      account.withdraw(100)
      expect(account.balance).toBe(2)
    })
  })
})

嗯，是的，看起来不错。不过，有些人可能已经注意到了：这里有代码异味。为什么这些测试共享一个account实例？这难道不会影响测试的顺序吗？如果我们交换顺序，代码肯定会崩溃。这不对。

describe('Account class', () => {
  describe('.deposit', () => {
    test('Should increment the account balance by the amount plus 2% incentive', () => {
      const account = new Account()
      account.deposit(100)
      expect(account.balance).toBe(102)
    })
  })

  describe('.withdraw', () => {
    test('Should decrement the account balance by the amount', () => {
      const account = new Account()
      account.withdraw(100)
      expect(account.balance).toBe(-100)
    })
  })
})

通过为每个测试创建account实例，可以确保测试从头开始。它可以根据需要进行任意修改，因为它包含在特定测试的范围内，并且彼此独立。这样，测试的顺序就无关紧要了。例如，如果我们使用并行运行并随机化测试顺序的测试运行器，它仍然可以顺利通过。

顺便说一句，我们还可以使用beforeEach/afterEach（或setup/teardown）助手来初始化和清理每个测试套件，但在这里解释起来相当复杂，所以也许需要另一篇文章来解释。

3. 没有正确维护国家

接下来，项目变得越来越大，显然有一些内部事务正在进行，现在所有代码都必须进行注释，将其放入适当的文件等等。

/**
 * Account class.
 */
class Account {
  /**
   * Constructor function.
   * 
   * This sets the initial balance when initializing the instance.
   * 
   * @param {Number} initialBalance 
   */
  constructor (initialBalance = 0) {
    this.balance = initialBalance
  }

  /**
   * Increment the balance by the given sum of the amount.
   * An incentive of 2% of the amount will be added
   * for each deposited amount.
   * 
   * @param {Number} money 
   */
  public deposit (money) {
    this.balance = (money * 1.02)
  }

  /**
   * Decrement the balance by the given amount.
   * 
   * @param {Number} money 
   */
  public withdraw (money) {
    this.balance -= money
  }
}

好了，完成了。我没有发现任何问题（或者我注意到了？😏 你很快就会发现的）。我查看了 Jest 控制台，它显示……

Account class
  .deposit
    ✓ Should increment the account balance by the amount plus 2% incentive (5ms)
  .withdraw
    ✓ Should decrement the account balance by the amount

显然还在通过。嗯。提交了，PR 审核了，CI 构建通过了，合并并部署了。真是一个愉快的星期一。

……但事实并非如此。用户抱怨他们的余额被重置为充值金额。这是怎么回事？测试都通过了，怎么会发生这种情况？

我检查了代码和测试，似乎没什么问题。是初始余额吗？因为我没有针对这个的测试（哎呀）。所以我继续更新测试，如下所示：

  describe('.deposit', () => {
    test('Should increment the account balance by the amount plus 2% incentive', () => {
      const account = new Account(100)

      account.deposit(100)
      expect(account.balance).toBe(202)
    })
  })

瞧瞧，不仅仅是用户，Jest 现在也在尖叫（？）

  ● Account class › .deposit › Should increment the account balance 
    by the amount plus 2% incentive

    expect(received).toBe(expected) // Object.is equality

    Expected: 202
    Received: 102

      11 |
      12 |       account.deposit(100)
    > 13 |       expect(account.balance).toBe(202)
         |                               ^
      14 |     })
      15 |   })
      16 |

      at Object.toBe (__tests__/index.test.js:13:31)

错误出现了！这正是用户报告的。现在测试真的失败了。查看代码后（你可以将其与本节开头的代码进行比较），我发现了一个小错误：

  deposit (money) {
    // The plus was missing 🤮
    this.balance += (money * 1.02)
  }

没错，就是这样。一个本该无害的重构竟然导致了一个 bug，可能是加号被意外删除了。测试没能发现它。我一开始就应该用正确的方式写。

如果代码是关于值累加（而不是值赋值）的，则必须以这样的方式进行测试：将先前的值与给定的值累加。之前的断言有点不完整，因为它只是测试了值赋值。

  // 🤔 
  describe('.deposit ❌', () => {
    test('Should increment the account balance by the amount plus 2% incentive', () => {
      const account = new Account() //... What's the previous value?

      account.deposit(100) // Amount is 100
      expect(account.balance).toBe(102) // Final value is 102...?
    })
  })

  // 😎
  describe('.deposit ✅', () => {
    test('Should increment the account balance by the amount plus 2% incentive', () => {
      const account = new Account(100) // Previous value is 100

      account.deposit(100) // Amount is 100
      expect(account.balance).toBe(202) // Final value is 202
    })
  })

为了确保万无一失，构造函数也必须进行测试。这确保实例化部分被正确覆盖（如果构造函数包含某些逻辑，也可以进行断言）。

  describe('constructor', () => {
    test('Should set the initial balance when instantiated', () => {
      const account = new Account(100)
      expect(account.balance).toBe(100)
    })
  })

或许这部分内容比较具体，但重点是，务必测试整个状态流程（之前/之后，I/O），而不是只测试部分。至少我的经验是这样的。

4. 测试结构不合理

我收到 QA 团队的反馈，说我没有正确捕获边缘情况。输入的值.deposit可以是任意值，但错误信息不够直观。

此外，还出现了新的要求：账户应该能够存入多个单笔金额，然后从中提取一笔款项。

好的。.deposit代码现在如下所示：

  /**
   * Increment the balance by the given sum of the amount.
   * An incentive of 2% of the amount will be added
   * for each deposited amount.
   * Only number is allowed, otherwise an error is thrown.
   * Also, the number should be greater than 0.
   * 
   * @param {Number[]} ...args 
   */
  deposit (...args) {
    if (args.length === 0) {
      throw new Error('Please provide at least one argument.')
    }

    const amount = args.reduce((total, value) => {
      const number = parseInt(value, 10)
      if (isNaN(number)) {
        throw new Error('Please specify a number as the argument.')
      }

      if (number <= 0) {
        throw new Error('Please specify a number greater than 0.')
      }

      return total + (number * 1.02)
    })

    this.balance += amount
  }

...但测试结果看起来不太好：

  describe('.deposit', () => {
    test('Should throw an error when no amount is given', () => {
      const account = new Account()
      expect(() => account.deposit()).toThrowError('Please provide at least one argument.')
    })

    test('Should throw an error when amount given is not a number', () => {
      const account = new Account()
      expect(() => account.deposit('a', 'b', 'c')).toThrowError('Please specify a number as the argument.')
    })

    test('Should increment the account balance by the sum of the amount plus 2% incentive, only when the amount is greater than 0 otherwise it should throw', () => {
      const account = new Account(100)

      account.deposit(100, 200)
      expect(account.balance).toBe(406)

      // ...but not less than 0!
      expect(() => account.deposit(0, 400, -200)).toThrowError('Please specify a number greater than 0.')
    })
  })

哇，测试的最后一部分真是拗口。算了🙄。工作完成了，测试也通过了。

  .deposit
    ✓ Should throw an error when no amount is given (4ms)
    ✓ Should throw an error when amount given is not a number (1ms)
    ✓ Should increment the account balance by the sum of the amount plus 2% incentive, only when the amount is greater than 0 otherwise it should throw (5ms)

然而，QA 团队说这个测试太乱了！很难理解，而且测试的最后一部分内容太多了。一般来说，最好把测试拆分成多个上下文，这样就可以有多层条件可以断言，并且一个测试应该只根据上下文做一件事。

改进的版本是：

  describe('.deposit', () => {
    describe('When no argument is provided', () => {
      test('Should throw an error', () => {
        const account = new Account()
        expect(() => account.deposit()).toThrowError('Please provide at least one argument.')
      })
    })

    describe('When the arguments are provided', () => {
      describe('And the arguments are invalid', () => {
        test('Should throw an error', () => {
          const account = new Account()
          expect(() => account.deposit('a', 'b', 'c')).toThrowError('Please specify a number as the argument.')
        })
      })

      describe('And the arguments are valid', () => {
        describe('And the arguments are less than zero', () => {
          test('Should throw an error', () => {
            const account = new Account()
            expect(() => account.deposit(0, 400, -200)).toThrowError('Please specify a number greater than 0.')
          })
        })

        describe('And the arguments are all more than zero', () => {
          test('Should increment the account balance by the sum of the amount plus 2% incentive', () => {
            const account = new Account(100)
            expect(account.balance).toBe(100)

            account.deposit(100, 200)
            expect(account.balance).toBe(406)
          })
        })
      })
    })
  })

当代码变得更加复杂时，多层上下文非常有用。当代码已经像这样分层时，添加更多上下文会更容易。例如，如果我要添加一个新的验证（例如应该设置最大存款金额），并且应该为此添加一个测试，我知道应该把它们放在结构中的哪个位置，这样既美观又整洁。

层的顺序主要取决于我的个人喜好。我喜欢将边缘情况放在顶层，将实际逻辑放在底层，有点像代码中守卫（或实际验证）的写法。

以下是 Jest 输出的样子：

  .deposit
    When no argument is provided
      ✓ Should throw an error (7ms)
    When the arguments are provided
      And the arguments are invalid
        ✓ Should throw an error (2ms)
      And the arguments are valid
        And the arguments are less than zero
          ✓ Should throw an error (2ms)
        And the arguments are all more than zero
          ✓ Should increment the account balance by the sum of the amount plus 2% incentive (2ms)

现在我不得不同意 QA 团队的意见。

5. 不信任你正在使用的库

利益相关者表示，有黑客以某种方式从账户中提取了不属于他们的资金。由于这个问题，该.withdraw函数不能简单地扣除余额；它必须经过一些验证脚本，才能知道它是否被黑客篡改（我不确定具体是如何做到的，这只是一个示例代码）。

  /**
   * Decrement the balance by the given amount.
   * It is now using a validator from backend
   * which I don't know how it works.
   * 
   * @param {Number} money 
   */
  withdraw (money) {
    const currentBalance = this.validateAndWithdraw(money)
    this.balance = currentBalance
  }

  validateAndWithdraw (money) {
    // This validator might throw an error if the transaction is invalid!!!
    return superDuperValidatorFromBackend(money)
  }

由于在 Jest 中实际运行成本很高，我宁愿模拟执行验证的函数。只要它不会抛出错误并返回实际余额，就可以了。

  describe('.withdraw', () => {
    describe('Given a valid withdrawal', () => {
      test('Should set the balance after withdrawal', () => {
        const account = new Account(300)

        // Override this function to avoid having to actually request from backend.
        // It should just return the balance without any error thrown.
        jest.spyOn(account, 'validateAndWithdraw').mockImplementationOnce(() => 200)

        expect(() => account.withdraw(100)).not.toThrow()
        expect(account.validateAndWithdraw).toHaveBeenCalledWith(100)
        expect(account.balance).toBe(200)
      })
    })
  })

我添加了not.toThrow()这个，这样我就可以知道当我调用这个.withdraw函数时，不会抛出任何错误，因为我已经模拟了它。对吧？对吧？

错误。——QA团队

最终，我明白了，我编写的测试应该只覆盖代码的业务逻辑。测试是否抛出错误不应该由我的测试负责，因为正如我在测试中指定的那样，函数实现已经被 Jest mocked，所以错误不会抛出。没有必要断言它是否应该抛出，因为它永远不会抛出！

...但是你怎么能这么确定呢？——我笨拙的测试技巧

你可以随时查看 Jest 的仓库、源代码以及他们的测试方法，看看它是否通过了。甚至可能还有确切的代码，谁知道呢。关键是，我必须信任我使用的库，确保他们的代码有效是他们的测试责任，而不是我的。我的测试应该专注于我代码的实际逻辑。

  describe('.withdraw', () => {
    describe('Given a valid withdrawal', () => {
      test('Should set the balance after withdrawal', () => {
        const account = new Account(300)

        // Override this function to avoid having to actually request from backend.
        // It should just return the balance without any error thrown.
        jest.spyOn(account, 'validateAndWithdraw').mockImplementationOnce(() => 200)

        account.withdraw(100)
        expect(account.validateAndWithdraw).toHaveBeenCalledWith(100)
        expect(account.balance).toBe(200)
      })
    })
  })

就是这样。只允许业务逻辑。

我的旅程就此结束，暂时告一段落。谁知道未来（错误）又会怎样呢……

另外，其中一些错误可能显而易见。但这些观点仍然成立。我只是想分享一下。如果您对这些建议有任何反馈，或者也许这根本不是什么错误，请在下方评论区留言，让我们一起讨论。

希望您喜欢阅读这篇文章，谢谢！

封面图片由Jamie Street在Unsplash上提供。

文章来源：https://dev.to/briwa/a-series-of-my-unfortunate-mistakes-when-writing-tests-h8m