AA test for Search Results #3053

Rosa-Fox · 2023-05-10T10:59:11Z

The purpose of the A/A test is to test the % of allocation of traffic/sample population based on the current version of elastic (6.7) against itself. A (ES 6.7) vs A (ES6.7). The A/A is solely to identify how the approach allocates traffic. This will provide benchmark data to aid analysis in future AA tests.

This test is only applied to /search/all results, not to finder search results.

Manual testing using Postman on /search/all for the different variants

A finder search results page, which the test is not applied to:

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

The purpose of the A/A test is to test the % of allocation of traffic/sample population based on the current version of elastic (6.7) against itself. A (ES 6.7) vs A (ES6.7). The A/A is solely to identify how the approach allocates traffic. This will provide benchmark data to aid analysis in future AA tests. Rendering app PR: alphagov/finder-frontend#3053

sihugh

As Leena mentioned, convention is to put the controller parts of the config in a concern. This test doesn't have any logic though, so it's probably OK. You might want the practise!

sihugh · 2023-07-04T09:23:17Z

app/controllers/finders_controller.rb

@@ -8,6 +8,16 @@ class FindersController < ApplicationController
  def show
    slimmer_template "gem_layout_full_width" if i_am_a_topic_page_finder

+    ab_test = GovukAbTesting::AbTest.new(


The AB test will run on every page shown by this action, which might not be desired if K&C just want results from /search/all.
You could add a page_under_test? method that tests whether the path is /search/all, and wrap this logic in that?

Thanks, I checked with them and they only want it on /search/all... which makes sense as this is what they will want to test with the real AB test! Thanks for flagging. I have updated it to use the constraint.

sihugh · 2023-07-04T09:24:00Z

app/views/finders/_show_header.html.erb

@@ -66,6 +66,10 @@
      <% end %>
    </div>

+    <% content_for :meta_tags do %>
+      <%= @requested_variant.analytics_meta_tag.html_safe %>


if you do add a page_under_test? method, you'd need to wrap this bit in it too

sihugh · 2023-07-04T09:24:24Z

app/controllers/finders_controller.rb

+      "EsSixPointSeven",
+      dimension: 1,
+      allowed_variants: %w[A B Z],
+      control_variant: "Z",


leenagupte

I think this looks ok. I've added a couple of suggestions for keeping the AB test code out of controller. I pretty much "borrowed" all suggestions from Kelvin 😅

leenagupte · 2023-07-04T17:24:06Z

app/controllers/finders_controller.rb

@@ -353,4 +359,8 @@ def filter_query_array(arr)
      arr.reject { |v| v == "all" }.compact.presence
    end
  end
+
+  def page_under_test?


I think this can be moved to the concern?

leenagupte · 2023-07-04T17:29:28Z

app/controllers/finders_controller.rb

+    if page_under_test?
+      @requested_variant = elastic_search_aa_test.requested_variant(request.headers)
+      @requested_variant.configure_response(response)
+    end
+


I think all of this can be moved to the concern too, to keep the controller tidy.

Rosa-Fox · 2023-07-10T13:56:03Z

Thanks for the suggestions @leenagupte. I have fixed up :)

leenagupte

This looks good. The concern is really nice.

I have one comment because I'm a bit confused by one of the tests.

leenagupte · 2023-07-11T17:25:51Z

spec/controllers/finders_controller_spec.rb

+            expect(response.body).to include("EsSixPointSeven:#{variant}")
+          end
+
+          it "doesn't render the #{variant} for finders" do


leenagupte · 2023-07-11T17:29:15Z

spec/controllers/finders_controller_spec.rb

+          end
+        end
+
+        it "should render the page without an AB test variant for search" do


I'm not sure I follow how this test is different from "renders the #{variant} variant for /search/all pages". Is it because the variant is not being passed in the header? But the Vary header is still being set?

yeah, tbh this was more out of paranoia to test that the page would still render if something went wrong with setting the variant, but its probably not needed as it should work!

The purpose of the A/A test is to test the % of allocation of traffic/sample population based on the current version of elastic (6.7) against itself. A (ES 6.7) vs A (ES6.7). The A/A is solely to identify how the approach allocates traffic. This will provide benchmark data to aid analysis in future AA tests. Rendering app PR: alphagov/finder-frontend#3053

The purpose of the A/A test is to test the % of allocation of traffic/sample population based on the current version of elastic (6.7) against itself. A (ES 6.7) vs A (ES6.7). The A/A is solely to identify how the approach allocates traffic. This will provide benchmark data to aid analysis in future AA tests. This test is only applied to /search/all results, not to finder search results.

gclssvglx

Looks great - thanks Rosa 👍

Removing this code as the AA test has been run. There is however a chance that we will run a throttled version of the same test in a few weeks... Given that it is being removed from the CDN config, I also decided to fully remove the configuration from here, as it will be quick enough to bring it back again in a few weeks if required. PR to implment the AA test config: #3053

We recently ran an AA test for the Search team (#3053). We are going to run it again, hence reinstating it in this PR! The purpose of the A/A test is to test the % of allocation of traffic/sample population based on the current version of elastic (6.7) against itself. A (ES 6.7) vs A (ES6.7). The A/A is solely to identify how the approach allocates traffic. This will provide benchmark data to aid analysis in future AA tests. This time around, the code will stay the same. We will use the same custom dimension and the variant split will still be 50/50. The only difference is that the custom dimension has been scoped as ‘user' as opposed to ‘session’ like it was last time. Scoping the custom dimension has already been done by a performance analyst. We plan to deploy this on Thursday 17 August for 1 week.

govuk-ci temporarily deployed to finder-frontend-pr-3053 May 10, 2023 10:59 Inactive

Rosa-Fox force-pushed the aa-test-es-6.7 branch from c5006f9 to 94ccde6 Compare May 10, 2023 11:00

govuk-ci temporarily deployed to finder-frontend-pr-3053 May 10, 2023 11:00 Inactive

Rosa-Fox force-pushed the aa-test-es-6.7 branch from 94ccde6 to 5faafb7 Compare July 3, 2023 15:17

Rosa-Fox mentioned this pull request Jul 3, 2023

ES 6.7 AA test config alphagov/govuk-cdn-config#463

Merged

sihugh reviewed Jul 4, 2023

View reviewed changes

Rosa-Fox force-pushed the aa-test-es-6.7 branch 3 times, most recently from ca70d08 to ea30764 Compare July 4, 2023 11:37

leenagupte reviewed Jul 4, 2023

View reviewed changes

Rosa-Fox force-pushed the aa-test-es-6.7 branch from ea30764 to 8191a0c Compare July 10, 2023 13:53

leenagupte reviewed Jul 11, 2023

View reviewed changes

Rosa-Fox force-pushed the aa-test-es-6.7 branch 3 times, most recently from 38587e2 to a2000d2 Compare July 13, 2023 13:22

Rosa-Fox force-pushed the aa-test-es-6.7 branch from a2000d2 to 397fe1e Compare July 17, 2023 09:50

Rosa-Fox changed the title ~~WIP - AA test for Search Results~~ AA test for Search Results Jul 17, 2023

Rosa-Fox marked this pull request as ready for review July 17, 2023 09:51

gclssvglx approved these changes Jul 17, 2023

View reviewed changes

Rosa-Fox merged commit 3f11092 into main Jul 18, 2023
7 checks passed

Rosa-Fox deleted the aa-test-es-6.7 branch July 18, 2023 13:29

Rosa-Fox mentioned this pull request Jul 27, 2023

Remove AA test config #3123

Merged

Rosa-Fox mentioned this pull request Aug 16, 2023

Reinstate AA test ElasticSearch 6.7 #3135

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AA test for Search Results #3053

AA test for Search Results #3053

Rosa-Fox commented May 10, 2023 •

edited

Loading

sihugh left a comment

sihugh Jul 4, 2023

Rosa-Fox Jul 4, 2023

sihugh Jul 4, 2023

Rosa-Fox Jul 4, 2023

sihugh Jul 4, 2023

leenagupte left a comment

leenagupte Jul 4, 2023

Rosa-Fox Jul 10, 2023

leenagupte Jul 4, 2023

Rosa-Fox Jul 10, 2023

Rosa-Fox commented Jul 10, 2023

leenagupte left a comment

leenagupte Jul 11, 2023

leenagupte Jul 11, 2023

Rosa-Fox Jul 12, 2023 •

edited

Loading

gclssvglx left a comment

AA test for Search Results #3053

AA test for Search Results #3053

Conversation

Rosa-Fox commented May 10, 2023 • edited Loading

Manual testing using Postman on /search/all for the different variants

A finder search results page, which the test is not applied to:

sihugh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leenagupte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rosa-Fox commented Jul 10, 2023

leenagupte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rosa-Fox Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

gclssvglx left a comment

Choose a reason for hiding this comment

Rosa-Fox commented May 10, 2023 •

edited

Loading

Rosa-Fox Jul 12, 2023 •

edited

Loading