Skip to content

Break repository file tests into three tests

Erick Banks requested to merge egb-refactor-repository-file into master

What does this MR do?

This MR is meant to start a discussion about in what style our tests should be written. As per the guidelines we split tests across multiple files (unless test setup is expensive). So, here, I've done that.

The advantages of doing this are:

  • Better parallelization. Multiple tests can be run in parallel, where the one large test before had to be run in serial.
  • Better test failure resolution. When the old test failed it wasn't immediately clear which operation failed: add?, update?, delete? Now if a failure occurs it is immediately clear where the problem is.
  • Adherence to documented advice.

The disadvantages are:

  • More lines of code in total due to setup for each test being repeated (especially for edit and delete).
  • We incur setup time costs for each file now (although this is somewhat abated by the increased parallelization).

The previous way of performing the test required a large it for the add, edit, delete test all in one block. In a previous refactor I encountered an it block that did both add and remove, and was able to split it into two clauses. However, because, as in this test, an item needed to be created before it could be removed I had to nest some contexts so that the test randomizer would not try to perform a remove before doing an add. My question to the department is: which style is preferred in this case? Do we do as in this MR and break the test into multiple files? Or do we do as in this linked MR and nest context to force the proper execution order?

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Erick Banks

Merge request reports