Skip to content

Commit

Permalink
Support brave-variations test seed testing process (#24604)
Browse files Browse the repository at this point in the history
Add --variations-pr flag support.
  • Loading branch information
goodov committed Jul 15, 2024
1 parent cb2ee90 commit b617693
Show file tree
Hide file tree
Showing 16 changed files with 230 additions and 33 deletions.
1 change: 1 addition & 0 deletions app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ source_set("unit_tests") {
deps = [
"//base",
"//base/test:test_support",
"//brave/components/variations:buildflags",
"//chrome/browser",
"//chrome/browser/companion/core",
"//chrome/browser/enterprise/connectors/analysis:features",
Expand Down
2 changes: 1 addition & 1 deletion browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ if (is_win && is_official_build) {
brave_chrome_browser_public_deps = [
"//brave/browser:browser_process",
"//brave/components/brave_sync:constants",
"//brave/components/variations:constants",
"//brave/components/variations",
]

if (is_mac) {
Expand Down
2 changes: 1 addition & 1 deletion chromium_src/chrome/app/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ specific_include_rules = {
"chrome_main_delegate.cc": [
"+brave/build/android/jni_headers/BraveQAPreferences_jni.h",
"+brave/components/brave_sync/buildflags.h",
"+brave/components/variations/buildflags.h",
"+brave/components/variations/command_line_utils.h",
"+components/dom_distiller/core/dom_distiller_switches.h",
"+components/signin/public/base/account_consistency_method.h",
"+components/sync/base/command_line_switches.h",
Expand Down
14 changes: 3 additions & 11 deletions chromium_src/chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "brave/app/brave_command_line_helper.cc"
#include "brave/app/brave_main_delegate.cc"
#include "brave/components/brave_sync/buildflags.h"
#include "brave/components/variations/buildflags.h"
#include "brave/components/variations/command_line_utils.h"
#include "build/build_config.h"
#include "components/dom_distiller/core/dom_distiller_switches.h"
#include "components/sync/base/command_line_switches.h"
Expand Down Expand Up @@ -78,16 +78,8 @@ std::optional<int> ChromeMainDelegate::BasicStartupComplete() {

command_line.AppendSwitchASCII(switches::kLsoUrl, kDummyUrl);

// Brave variations
command_line.AppendSwitchASCII(variations::switches::kVariationsServerURL,
BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL));
// Insecure fall-back for variations is set to the same (secure) URL. This is
// done so that if VariationsService tries to fall back to insecure url the
// check for kHttpScheme in VariationsService::MaybeRetryOverHTTP would
// prevent it from doing so as we don't want to use an insecure fall-back.
command_line.AppendSwitchASCII(
variations::switches::kVariationsInsecureServerURL,
BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL));
variations::AppendBraveCommandLineOptions(
*base::CommandLine::ForCurrentProcess());

// Runtime-enabled features. To override Chromium features default state
// please see: brave/chromium_src/base/feature_override.h
Expand Down
7 changes: 6 additions & 1 deletion chromium_src/components/version_ui/version_handler_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ base::Value::List GetVariationsList() {
}

base::Value::List variations_list;
for (std::string& variation : variations)
const std::string& seed_version = variations::GetSeedVersion();
if (!seed_version.empty() && seed_version != "1") {
variations_list.Append(seed_version);
}
for (std::string& variation : variations) {
variations_list.Append(std::move(variation));
}

return variations_list;
}
Expand Down
28 changes: 26 additions & 2 deletions components/variations/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,30 @@ buildflag_header("buildflags") {
flags = [ "BRAVE_VARIATIONS_SERVER_URL=\"$brave_variations_server_url\"" ]
}

group("constants") {
public_deps = [ ":buildflags" ]
static_library("variations") {
sources = [
"command_line_utils.cc",
"command_line_utils.h",
"switches.h",
]

deps = [
":buildflags",
"//base",
"//components/variations",
]
}

source_set("unit_tests") {
testonly = true

sources = [ "command_line_utils_unittest.cc" ]

deps = [
":buildflags",
":variations",
"//base",
"//components/variations",
"//testing/gtest",
]
}
63 changes: 63 additions & 0 deletions components/variations/command_line_utils.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright (c) 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

#include "brave/components/variations/command_line_utils.h"

#include <string>

#include "base/command_line.h"
#include "base/strings/string_util.h"
#include "brave/components/variations/buildflags.h"
#include "brave/components/variations/switches.h"
#include "components/variations/variations_switches.h"

namespace variations {

namespace {

// A GitHub workflow in the brave/brave-variations repository generates the test
// seed and uploads it to a URL with the following template, where $1 is the
// pull request number.
constexpr char kVariationsPrTestSeedUrlTemplate[] =
"https://griffin.brave.com/pull/$1/seed";

} // namespace

void AppendBraveCommandLineOptions(base::CommandLine& command_line) {
std::string variations_server_url = BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL);

if (command_line.HasSwitch(switches::kVariationsPR)) {
variations_server_url = base::ReplaceStringPlaceholders(
kVariationsPrTestSeedUrlTemplate,
{command_line.GetSwitchValueASCII(switches::kVariationsPR)}, nullptr);

// Generated seed is not signed, so we need to disable signature check.
command_line.AppendSwitch(
variations::switches::kAcceptEmptySeedSignatureForTesting);

// Disable fetch throttling to force the fetch at startup on mobile
// platforms.
command_line.AppendSwitch(
variations::switches::kDisableVariationsSeedFetchThrottling);
}

if (!command_line.HasSwitch(variations::switches::kVariationsServerURL)) {
command_line.AppendSwitchASCII(variations::switches::kVariationsServerURL,
variations_server_url);
}

// Insecure fall-back for variations is set to the same (secure) URL. This
// is done so that if VariationsService tries to fall back to insecure url
// the check for kHttpScheme in VariationsService::MaybeRetryOverHTTP would
// prevent it from doing so as we don't want to use an insecure fall-back.
if (!command_line.HasSwitch(
variations::switches::kVariationsInsecureServerURL)) {
command_line.AppendSwitchASCII(
variations::switches::kVariationsInsecureServerURL,
variations_server_url);
}
}

} // namespace variations
21 changes: 21 additions & 0 deletions components/variations/command_line_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

#ifndef BRAVE_COMPONENTS_VARIATIONS_COMMAND_LINE_UTILS_H_
#define BRAVE_COMPONENTS_VARIATIONS_COMMAND_LINE_UTILS_H_

namespace base {
class CommandLine;
}

namespace variations {

// Appends Brave-specific command line options to fetch variations seed from the
// correct server.
void AppendBraveCommandLineOptions(base::CommandLine& command_line);

} // namespace variations

#endif // BRAVE_COMPONENTS_VARIATIONS_COMMAND_LINE_UTILS_H_
70 changes: 70 additions & 0 deletions components/variations/command_line_utils_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

#include "brave/components/variations/command_line_utils.h"

#include <string>

#include "base/command_line.h"
#include "brave/components/variations/buildflags.h"
#include "brave/components/variations/switches.h"
#include "components/variations/variations_switches.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace variations {

TEST(VariationsCommandLineUtils, DefaultVariationsServerUrl) {
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
AppendBraveCommandLineOptions(command_line);

EXPECT_EQ(command_line.GetSwitchValueASCII(
variations::switches::kVariationsServerURL),
BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL));
EXPECT_EQ(command_line.GetSwitchValueASCII(
variations::switches::kVariationsInsecureServerURL),
BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL));
EXPECT_FALSE(command_line.HasSwitch(
variations::switches::kAcceptEmptySeedSignatureForTesting));
EXPECT_FALSE(command_line.HasSwitch(
variations::switches::kDisableVariationsSeedFetchThrottling));
}

TEST(VariationsCommandLineUtils, OverrideVariationsServerUrl) {
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
const std::string override_variations_url = "https://variations.com";
const std::string override_insecure_variations_url = "http://insecure.com";
command_line.AppendSwitchASCII(variations::switches::kVariationsServerURL,
override_variations_url);
command_line.AppendSwitchASCII(
variations::switches::kVariationsInsecureServerURL,
override_insecure_variations_url);
AppendBraveCommandLineOptions(command_line);

EXPECT_EQ(override_variations_url,
command_line.GetSwitchValueASCII(
variations::switches::kVariationsServerURL));
EXPECT_EQ(override_insecure_variations_url,
command_line.GetSwitchValueASCII(
variations::switches::kVariationsInsecureServerURL));
}

TEST(VariationsCommandLineUtils, SetVariationsPrParameter) {
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
command_line.AppendSwitchASCII(variations::switches::kVariationsPR, "1234");
AppendBraveCommandLineOptions(command_line);

EXPECT_EQ(command_line.GetSwitchValueASCII(
variations::switches::kVariationsServerURL),
"https://griffin.brave.com/pull/1234/seed");
EXPECT_EQ(command_line.GetSwitchValueASCII(
variations::switches::kVariationsInsecureServerURL),
"https://griffin.brave.com/pull/1234/seed");
EXPECT_TRUE(command_line.HasSwitch(
variations::switches::kAcceptEmptySeedSignatureForTesting));
EXPECT_TRUE(command_line.HasSwitch(
variations::switches::kDisableVariationsSeedFetchThrottling));
}

} // namespace variations
18 changes: 18 additions & 0 deletions components/variations/switches.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

#ifndef BRAVE_COMPONENTS_VARIATIONS_SWITCHES_H_
#define BRAVE_COMPONENTS_VARIATIONS_SWITCHES_H_

namespace variations::switches {

// If this flag is set to a brave/brave-variations pull request number, the
// browser will point "--variations-server-url" to a test seed URL from this
// pull request.
inline constexpr char kVariationsPR[] = "variations-pr";

} // namespace variations::switches

#endif // BRAVE_COMPONENTS_VARIATIONS_SWITCHES_H_
2 changes: 1 addition & 1 deletion ios/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ source_set("app") {
"//brave/components/p3a:buildflags",
"//brave/components/skus/browser",
"//brave/components/update_client:buildflags",
"//brave/components/variations:constants",
"//brave/components/variations",
"//brave/ios/app/resources",
"//brave/ios/browser",
"//brave/ios/browser/api/ads",
Expand Down
5 changes: 5 additions & 0 deletions ios/app/brave_core_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ OBJC_EXPORT const BraveCoreSwitchKey BraveCoreSwitchKeyVModule;
///
/// Expected value: A URL string
OBJC_EXPORT const BraveCoreSwitchKey BraveCoreSwitchKeySyncURL;
/// Sets the variations URL to a test seed generated in brave/brave-variations
/// pull request.
///
/// Expected value: A brave/brave-variations pull request number
OBJC_EXPORT const BraveCoreSwitchKey BraveCoreSwitchKeyVariationsPR;
/// Overrides the variations seed URL. Defaults to production
///
/// Expected value: A URL string
Expand Down
3 changes: 3 additions & 0 deletions ios/app/brave_core_switches.mm
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/strings/sys_string_conversions.h"
#include "brave/components/brave_component_updater/browser/switches.h"
#include "brave/components/p3a/switches.h"
#include "brave/components/variations/switches.h"
#include "components/component_updater/component_updater_switches.h"
#include "components/sync/base/command_line_switches.h"
#include "components/variations/variations_switches.h"
Expand All @@ -23,6 +24,8 @@
base::SysUTF8ToNSString(switches::kVModule);
const BraveCoreSwitchKey BraveCoreSwitchKeySyncURL =
base::SysUTF8ToNSString(syncer::kSyncServiceURL);
const BraveCoreSwitchKey BraveCoreSwitchKeyVariationsPR =
base::SysUTF8ToNSString(variations::switches::kVariationsPR);
const BraveCoreSwitchKey BraveCoreSwitchKeyVariationsURL =
base::SysUTF8ToNSString(variations::switches::kVariationsServerURL);
// There is no exposed switch for rewards
Expand Down
16 changes: 2 additions & 14 deletions ios/app/brave_main_delegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include "brave/components/brave_component_updater/browser/switches.h"
#include "brave/components/brave_sync/buildflags.h"
#include "brave/components/update_client/buildflags.h"
#include "brave/components/variations/buildflags.h"
#include "brave/components/variations/command_line_utils.h"
#include "components/browser_sync/browser_sync_switches.h"
#include "components/component_updater/component_updater_switches.h"
#include "components/sync/base/command_line_switches.h"
Expand Down Expand Up @@ -72,19 +72,7 @@ @implementation BundleLookupClass
BUILDFLAG(BRAVE_SYNC_ENDPOINT));
}

// Brave variations
if (!command_line->HasSwitch(variations::switches::kVariationsServerURL)) {
command_line->AppendSwitchASCII(variations::switches::kVariationsServerURL,
BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL));

// Insecure fall-back for variations is set to the same (secure) URL. This
// is done so that if VariationsService tries to fall back to insecure url
// the check for kHttpScheme in VariationsService::MaybeRetryOverHTTP would
// prevent it from doing so as we don't want to use an insecure fall-back.
command_line->AppendSwitchASCII(
variations::switches::kVariationsInsecureServerURL,
BUILDFLAG(BRAVE_VARIATIONS_SERVER_URL));
}
variations::AppendBraveCommandLineOptions(*command_line);

if (!command_line->HasSwitch(switches::kVModule)) {
command_line->AppendSwitchASCII(switches::kVModule, "*/brave/*=0");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ extension BraveCoreSwitchKey {
return "Component Updater"
case .syncURL:
return "Sync URL"
case .variationsPR:
return "Variations PR"
case .variationsURL:
return "Variations URL"
case .p3aDoNotRandomizeUploadInterval:
Expand Down Expand Up @@ -317,14 +319,18 @@ struct BraveCoreDebugSwitchesView: View {
}
Section {
Group {
// Sync URL
NavigationLink {
BasicStringInputView(coreSwitch: .syncURL)
.keyboardType(.URL)
} label: {
SwitchContainer(.syncURL)
}
// Variations URL
NavigationLink {
BasicStringInputView(coreSwitch: .variationsPR)
.keyboardType(.numberPad)
} label: {
SwitchContainer(.variationsPR)
}
NavigationLink {
BasicStringInputView(coreSwitch: .variationsURL)
.keyboardType(.URL)
Expand Down
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ test("brave_unit_tests") {
"//brave/components/time_period_storage",
"//brave/components/tor/buildflags",
"//brave/components/url_sanitizer/browser:unittests",
"//brave/components/variations:unit_tests",
"//brave/components/version_info:unit_tests",
"//brave/extensions:common",
"//brave/mojo/brave_ast_patcher:unit_tests",
Expand Down

0 comments on commit b617693

Please sign in to comment.