Prevent the version to be Brave-only (without the Chromium major). (#…

…1240) Prevent situations where the version filter is set to a Brave version, such as: `min_version: '1.72.51'`.
brave · Oct 25, 2024 · 885cb07 · 885cb07
1 parent 7252645
commit 885cb07
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 58 deletions.
diff --git a/src/seed_tools/utils/seed_validation.test.ts b/src/seed_tools/utils/seed_validation.test.ts
@@ -130,52 +130,52 @@ describe('getSeedErrors', () => {
       // min/max version tests.
       {
         filter1: {
-          min_version: '1.0.0',
-          max_version: '2.0.0',
+          min_version: '100.1.0.0',
+          max_version: '100.2.0.0',
         },
         filter2: {
-          min_version: '1.5.0',
-          max_version: '2.5.0',
+          min_version: '100.1.5.0',
+          max_version: '100.2.5.0',
         },
         expectedOverlapped: true,
       },
       {
         filter1: {
-          min_version: '1.0.0',
-          max_version: '2.0.0',
+          min_version: '100.1.0.0',
+          max_version: '100.2.0.0',
         },
         filter2: {
-          min_version: '1.*',
-          max_version: '2.*',
+          min_version: '100.1.*',
+          max_version: '100.2.*',
         },
         expectedOverlapped: true,
       },
       {
         filter1: {
-          min_version: '1.0.0',
-          max_version: '2.0.0',
+          min_version: '100.1.0.0',
+          max_version: '100.2.0.0',
         },
         filter2: {
-          min_version: '2.1.*',
-          max_version: '2.*',
+          min_version: '100.2.1.*',
+          max_version: '100.2.*',
         },
         expectedOverlapped: false,
       },
       {
         filter1: {
-          min_version: '1.5.0',
-          max_version: '2.0.0',
+          min_version: '100.1.5.0',
+          max_version: '100.2.0.0',
         },
         filter2: {
-          min_version: '1.*',
-          max_version: '1.5.*',
+          min_version: '100.1.*',
+          max_version: '100.1.5.*',
         },
         expectedOverlapped: true,
       },
       {
         filter1: {
-          min_version: '1.5.0',
-          max_version: '2.0.0',
+          min_version: '100.1.5.0',
+          max_version: '100.2.0.0',
         },
         filter2: {},
         expectedOverlapped: true,

diff --git a/src/seed_tools/utils/study_validation.test.ts b/src/seed_tools/utils/study_validation.test.ts
@@ -596,47 +596,51 @@ describe('getStudyErrors', () => {
     );
   });
 
-  test.each([{ min_version: '130.0.6517.0' }, { max_version: '135.0.6707.0' }])(
-    'should error if version is Chromium %s',
-    (filter: any) => {
-      const study = Study.fromJson({
-        name: 'study',
-        experiment: [
-          {
-            name: 'experiment1',
-            probability_weight: 100,
-          },
-        ],
-        filter,
-      });
+  test.each([
+    { min_version: '130.0.6517.0' },
+    { max_version: '135.0.6707.0' },
+    { min_version: '1.65.70' },
+    { min_version: '82.0.4056.0' },
+    { min_version: '79.0.3945.0' },
+  ])('should error if version is non-Brave %s', (filter: any) => {
+    const study = Study.fromJson({
+      name: 'study',
+      experiment: [
+        {
+          name: 'experiment1',
+          probability_weight: 100,
+        },
+      ],
+      filter,
+    });
 
-      expect(
-        study_validation.getStudyErrors(study, studyFileBaseName),
-      ).toContainEqual(
-        expect.stringContaining('Detected Chromium version in a filter'),
-      );
-    },
-  );
+    expect(
+      study_validation.getStudyErrors(study, studyFileBaseName),
+    ).toContainEqual(
+      expect.stringContaining('Detected non-Brave version in a filter'),
+    );
+  });
 
-  test.each([{ min_version: '130.1.70.0' }, { max_version: '135.1.91.0' }])(
-    'should not error if version is Brave %s',
-    (filter: any) => {
-      const study = Study.fromJson({
-        name: 'study',
-        experiment: [
-          {
-            name: 'experiment1',
-            probability_weight: 100,
-          },
-        ],
-        filter: { ...filter, platform: ['PLATFORM_LINUX'], channel: ['BETA'] },
-      });
+  test.each([
+    { min_version: '130.1.70.0' },
+    { max_version: '135.1.91.0' },
+    { min_version: '80.1.8.1' },
+  ])('should not error if version is Brave %s', (filter: any) => {
+    const study = Study.fromJson({
+      name: 'study',
+      experiment: [
+        {
+          name: 'experiment1',
+          probability_weight: 100,
+        },
+      ],
+      filter: { ...filter, platform: ['PLATFORM_LINUX'], channel: ['BETA'] },
+    });
 
-      expect(study_validation.getStudyErrors(study, studyFileBaseName)).toEqual(
-        [],
-      );
-    },
-  );
+    expect(study_validation.getStudyErrors(study, studyFileBaseName)).toEqual(
+      [],
+    );
+  });
 
   test('should error if os version range is invalid', () => {
     const study = Study.fromJson({

diff --git a/src/seed_tools/utils/study_validation.ts b/src/seed_tools/utils/study_validation.ts
@@ -239,10 +239,10 @@ function checkVersionRange(study: Study): string[] {
     if (
       version !== undefined &&
       version.components.length >= 3 &&
-      version.components[2] > 6000
+      (version.components[0] < 80 || version.components[2] > 4000)
     ) {
       errors.push(
-        `Detected Chromium version in a filter for study ${study.name}: ${version.toString()}. Use Brave version in a format CHROMIUM_MAJOR.BRAVE_MAJOR.BRAVE_MINOR.BRAVE_BUILD`,
+        `Detected non-Brave version in a filter for study ${study.name}: ${version.toString()}. Use Brave version in a format CHROMIUM_MAJOR.BRAVE_MAJOR.BRAVE_MINOR.BRAVE_BUILD`,
       );
     }
   };