Merge pull request #19 from sundeep/fix_expt_name_case_insensitivity

Update validity check for experiment names to be case sensitive
sixpack · Jun 7, 2016 · b2ed020 · b2ed020
2 parents af1c7a8 + 41f1401
commit b2ed020
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 3 deletions.
diff --git a/sixpack/sixpack.py b/sixpack/sixpack.py
@@ -4,6 +4,12 @@
 
 SIXPACK_HOST = 'http://localhost:5000'
 SIXPACK_TIMEOUT = 0.5
+
+# valid experiment_names must be lowercase, start with an alphanumeric and
+# contain alphanumerics, dashes and underscores
+VALID_EXPT_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9\-_ ]*$")
+
+# Valid alternative and kpi names
 VALID_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9\-_ ]*$", re.I)
 
 
@@ -42,7 +48,7 @@ def __init__(self, client_id=None, options=None, params=None):
             self.client_id = client_id
 
     def participate(self, experiment_name, alternatives, force=None, traffic_fraction=1, prefetch=False):
-        if VALID_NAME_RE.match(experiment_name) is None:
+        if VALID_EXPT_NAME_RE.match(experiment_name) is None:
             raise ValueError('Bad experiment name')
 
         if len(alternatives) < 2:
@@ -72,7 +78,7 @@ def participate(self, experiment_name, alternatives, force=None, traffic_fractio
         return response
 
     def convert(self, experiment_name, kpi=None):
-        if VALID_NAME_RE.match(experiment_name) is None:
+        if VALID_EXPT_NAME_RE.match(experiment_name) is None:
             raise ValueError('Bad experiment name')
 
         params = {

diff --git a/sixpack/test/test_sixpack_client.py b/sixpack/test/test_sixpack_client.py
@@ -59,7 +59,7 @@ def test_client_id_on_constructor(self):
         session = sixpack.Session(client_id='zack111')
         self.assertEqual(session.client_id, 'zack111')
 
-    def test_failure_on_bad_experiment_name(self):
+    def test_failure_on_bad_experiment_name_participate(self):
         session = sixpack.Session('zack')
         with self.assertRaises(ValueError):
             session.participate('*********', ['1', '2'])
@@ -69,6 +69,16 @@ def test_failure_on_bad_exp_name_convert(self):
         with self.assertRaises(ValueError):
             session.convert('(((((')
 
+    def test_failure_on_uppercase_expt_name_participate(self):
+        session = sixpack.Session('zack')
+        with self.assertRaises(ValueError):
+            session.participate('FooExperiment', ['1', '2'])
+
+    def test_failure_on_uppercase_expt_name_convert(self):
+        session = sixpack.Session('zack')
+        with self.assertRaises(ValueError):
+            session.convert('FooExperiment')
+
     def test_failure_on_bad_kpi_name_convert(self):
         session = sixpack.Session('zack')
         with self.assertRaises(ValueError):