From ed61c068c91bdecf65f0171904193ee17349e483 Mon Sep 17 00:00:00 2001
From: Robert May <rmay@gitlab.com>
Date: Thu, 20 Aug 2020 07:37:48 +0000
Subject: [PATCH] Make Elastic::ProcessBookkeepingService specs work on Redis 4

The service itself doesn't use ZPOPMIN and friends, but the spec did.
Changing this to the slightly less convenient ZRANGE ensures that the
specs will run on Redis 4, which is our minimum supported version.
---
 .gitlab/ci/global.gitlab-ci.yml                        |  8 ++++----
 doc/development/pipelines.md                           |  2 +-
 .../elastic/process_bookkeeping_service_spec.rb        | 10 +++++-----
 .../gitlab/instrumentation/redis_interceptor_spec.rb   |  4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml
index ae8db21c99c26..238059bf97225 100644
--- a/.gitlab/ci/global.gitlab-ci.yml
+++ b/.gitlab/ci/global.gitlab-ci.yml
@@ -68,7 +68,7 @@
   services:
     - name: postgres:11.6
       command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"]
-    - name: redis:alpine
+    - name: redis:4.0-alpine
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
 
@@ -77,7 +77,7 @@
   services:
     - name: postgres:12
       command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"]
-    - name: redis:alpine
+    - name: redis:4.0-alpine
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
 
@@ -86,7 +86,7 @@
   services:
     - name: postgres:11.6
       command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"]
-    - name: redis:alpine
+    - name: redis:4.0-alpine
     - name: elasticsearch:6.4.2
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
@@ -96,7 +96,7 @@
   services:
     - name: postgres:12
       command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"]
-    - name: redis:alpine
+    - name: redis:4.0-alpine
     - name: elasticsearch:6.4.2
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md
index 0886c96e4869c..aef14535a96ef 100644
--- a/doc/development/pipelines.md
+++ b/doc/development/pipelines.md
@@ -585,7 +585,7 @@ that are scoped to a single [configuration parameter](../ci/yaml/README.md#confi
 | `.static-analysis-cache` | Allows a job to use a default `cache` definition suitable for static analysis tasks. |
 | `.yarn-cache` | Allows a job to use a default `cache` definition suitable for frontend jobs that do a `yarn install`. |
 | `.assets-compile-cache` | Allows a job to use a default `cache` definition suitable for frontend jobs that compile assets. |
-| `.use-pg11` | Allows a job to use the `postgres:11.6` and `redis:alpine` services. |
+| `.use-pg11` | Allows a job to use the `postgres:11.6` and `redis:4.0-alpine` services. |
 | `.use-pg11-ee` | Same as `.use-pg11` but also use the `docker.elastic.co/elasticsearch/elasticsearch:6.4.2` services. |
 | `.use-kaniko` | Allows a job to use the `kaniko` tool to build Docker images. |
 | `.as-if-foss` | Simulate the FOSS project by setting the `FOSS_ONLY='1'` environment variable. |
diff --git a/ee/spec/services/elastic/process_bookkeeping_service_spec.rb b/ee/spec/services/elastic/process_bookkeeping_service_spec.rb
index 0701dae74e00e..dd2a9c4f022a5 100644
--- a/ee/spec/services/elastic/process_bookkeeping_service_spec.rb
+++ b/ee/spec/services/elastic/process_bookkeeping_service_spec.rb
@@ -22,7 +22,7 @@
     it 'enqueues a record' do
       described_class.track!(issue)
 
-      spec, score = redis.zpopmin(zset)
+      spec, score = redis.zrange(zset, 0, 0, with_scores: true).first
 
       expect(spec).to eq(issue_spec)
       expect(score).to eq(1.0)
@@ -33,8 +33,7 @@
 
       expect(described_class.queue_size).to eq(fake_refs.size)
 
-      spec1, score1 = redis.zpopmin(zset)
-      _, score2 = redis.zpopmin(zset)
+      (spec1, score1), (_, score2), _ = redis.zrange(zset, 0, -1, with_scores: true)
 
       expect(score1).to be < score2
       expect(spec1).to eq(issue_spec)
@@ -61,7 +60,7 @@
 
       expect(described_class.queue_size).to eq(fake_refs.size)
 
-      expect { redis.zpopmin(zset) }.to change(described_class, :queue_size).by(-1)
+      expect { redis.zadd(zset, 0, 'foo') }.to change(described_class, :queue_size).by(1)
     end
   end
 
@@ -116,7 +115,8 @@
 
       expect { described_class.new.execute }.to change(described_class, :queue_size).by(-limit + 1)
 
-      serialized, _ = redis.zpopmax(zset)
+      serialized = redis.zrange(zset, -1, -1).first
+
       expect(ref_class.deserialize(serialized)).to eq(failed)
     end
 
diff --git a/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb b/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb
index 5b0ad63ee72b2..09280402e2b21 100644
--- a/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb
+++ b/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb
@@ -25,8 +25,8 @@
       # Exercise counting of a bulk reply
       [[:set, 'foo', 'bar' * 100]] | [:get, 'foo'] | 3 + 3 | 3 * 100
 
-      # Nested array response: ['123456-89', ['foo', 'bar']]
-      [[:xadd, 'mystream', '123456-89', 'foo', 'bar']] | [:xrange, 'mystream', '-', '+'] | 6 + 8 + 1 + 1 | 9 + 3 + 3
+      # Nested array response: [['foo', 0], ['bar', 1]]
+      [[:zadd, 'myset', 0, 'foo'], [:zadd, 'myset', 1, 'bar']] | [:zrange, 'myset', 0, -1, 'withscores'] | 6 + 5 + 1 + 2 + 10 | 3 + 1 + 3 + 1
     end
 
     with_them do
-- 
GitLab