Skip to content

Fix Feature Flag Controller Spec Sorting on Name

Jason Goodman requested to merge fix-ff-spec-name-sorting into master

What does this MR do?

Fix a sorting issue in feature flag controller specs.

We sort feature flags by name for GET requests to the index:

The feature flag factory generates a random name of the form feature_flag_#{n}:

Our specs for the FeatureFlagController#index assume that the flags will be sorted in the order they are created. Most of the time, this will be true. Consider the case when we generate names like feature_flag_79, feature_flag_80, feature_flag_81, and so on:

gitlabhq_development=# SELECT id, name FROM (VALUES (79, 'feature_flag_79'), (80, 'feature_flag_80'), (81, 'feature_flag_81')) AS t(id, name) ORDER BY name;
 id |      name       
----+-----------------
 79 | feature_flag_79
 80 | feature_flag_80
 81 | feature_flag_81
(3 rows)

gitlabhq_development=#

However, we may generate feature flag names like feature_flag_99, feature_flag_100, in which case Postgres will sort the second created flag first:

gitlabhq_development=# SELECT id, name FROM (VALUES (99, 'feature_flag_99'), (100, 'feature_flag_100')) AS t(id, name) ORDER BY name;
 id  |       name       
-----+------------------
 100 | feature_flag_100
  99 | feature_flag_99
(2 rows)

gitlabhq_development=#

If we run enough specs thereby bumping up the sequence number in the factory, there is a chance that this generates names that suddenly sort in an order unexpected by the specs, causing a spec to randomly fail.

I ran into this locally while working on #212318 (closed).

I can consistently cause this spec fail to by running bin/rspec ee/spec/controllers/projects/feature_flag_issues_controller_spec.rb ee/spec/controllers/projects/feature_flags_controller_spec.rb, because the two flags in the spec are generated with the names feature_flag_99 and feature_flag_100.

This MR fixes the issue by giving the flags created in the specs consistent, rather than randomly generated, names so they should always sort in the same order.

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 Jason Goodman

Merge request reports