Skip to content

Exhaustively test relative positioning logic

Alex Kalderimis requested to merge ajk-relative-positioning-mover into master

What does this MR do?

The best way to get high assurance of a system is property based testing, where properties are asserted given a defined set of input states. This is currently not feasible for our relative positioning system, since the number of states is astronomical in size (the #move_after operation can have 2^4bn distinct input states, for example).

To reduce this and enable full test coverage, this MR:

  • extracts the core logic to two helper classes: Gitlab::RelativePositioning::Mover and Gitlab::RelativePositioning::ItemContext, along with two smaller abstractions Range and `Gap.
  • These objects can be instantiated parameterized by range and start position (currently fixed constant values)
  • The public methods of RelativePositioning (move_after, move_between, move_before, move_to_end, move_to_start) are implemented in terms of these primitives

This has the benefit that:

  • we can test exhaustively over a smaller range, one that is tractable
  • we no longer need to pollute the namespaces of including classes with irrelevant private methods

In the course of this exercise a number of subtle edge case bugs were addressed, and we can now have confidence that the core movement logic is sound (excepting query time-outs).

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

This makes the testing for relative positioning more thorough. It does not remove tests, or change their operational semantics.

Some tests for private members have been removed, since these are no longer accessible.

Edited by Alex Kalderimis

Merge request reports