From c95d88deae5d654af94f13cf7c492af642933575 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 13:02:54 -0500 Subject: [PATCH 01/15] Don't allow fake users --- rules/0.17-allow-new-players.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 0d3a827..881d0cd 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -15,6 +15,10 @@ def should_allow(pr): if points_user in util.users(): raise Exception('Cannot create an existing user') + + response = util.request(_user_url(points_user)) + if (response.status_code != 200) + raise Exception('%s is not a real GitHub user' % points_user) if points_name != 'initial': raise Exception('New player bonus value must be called "initial"') @@ -27,3 +31,6 @@ def should_allow(pr): (points_change, max_start_bonus)) return True + +def _user_url(user: str) -> str: + return 'https://www.jefftk.com/nomic-github/users/%s' % user From fd4aae97f5624c75f679064c63fc53b162180d79 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 13:04:27 -0500 Subject: [PATCH 02/15] No real reason to make the url its own method --- rules/0.17-allow-new-players.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 881d0cd..c5e16c5 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -16,7 +16,7 @@ def should_allow(pr): if points_user in util.users(): raise Exception('Cannot create an existing user') - response = util.request(_user_url(points_user)) + response = util.request('https://www.jefftk.com/nomic-github/users/%s' % points_user) if (response.status_code != 200) raise Exception('%s is not a real GitHub user' % points_user) @@ -31,6 +31,3 @@ def should_allow(pr): (points_change, max_start_bonus)) return True - -def _user_url(user: str) -> str: - return 'https://www.jefftk.com/nomic-github/users/%s' % user From b2e6d7c844ecb3f890a35a77491f2ac89f6affd1 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 13:28:20 -0500 Subject: [PATCH 03/15] Better check for blocking existing players. Also re-order to put cheaper checks first. --- rules/0.17-allow-new-players.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index c5e16c5..0fa0634 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -13,21 +13,24 @@ def should_allow(pr): (points_user, points_name, points_change) = bonuses[0] - if points_user in util.users(): - raise Exception('Cannot create an existing user') - - response = util.request('https://www.jefftk.com/nomic-github/users/%s' % points_user) - if (response.status_code != 200) - raise Exception('%s is not a real GitHub user' % points_user) - if points_name != 'initial': - raise Exception('New player bonus value must be called "initial"') - + raise Exception('New player bonus value is called %s instead of "initial"' % points_name) + if points_change < 0: raise Exception('Points cannot be negative') if points_change > max_start_bonus: raise Exception('%s initial points exceeds maximum starting value of %s points' % (points_change, max_start_bonus)) + + bonus_directory = os.path.join('players', points_user, 'bonuses') + if os.path.isdir(bonus_directory): + for bonus in os.listdir(bonus_directory): + if bonus != 'initial': + raise Exception('%s already has bonuses' % points_user) + + response = util.request('https://www.jefftk.com/nomic-github/users/%s' % points_user) + if (response.status_code != 200) + raise Exception('%s is not a real GitHub user' % points_user) return True From 9c4404814d6f6b3e5f8917ae4ed49a42d624b293 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 13:35:41 -0500 Subject: [PATCH 04/15] Oops --- rules/0.17-allow-new-players.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 0fa0634..dfef669 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -30,7 +30,7 @@ def should_allow(pr): raise Exception('%s already has bonuses' % points_user) response = util.request('https://www.jefftk.com/nomic-github/users/%s' % points_user) - if (response.status_code != 200) + if response.status_code != 200: raise Exception('%s is not a real GitHub user' % points_user) return True From 1e17c743a7a0a080959230a5098da7fb8ffe45a0 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 13:36:26 -0500 Subject: [PATCH 05/15] Missing import --- rules/0.17-allow-new-players.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index dfef669..47f9196 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -1,3 +1,4 @@ +import os import util # This value set to 0 until we figure out a good way to prevent dummy account abuse From 32211a47351302a02bc678b7366b6ddff7ce0cc3 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 14:01:59 -0500 Subject: [PATCH 06/15] Validate that a new user created their own PR --- rules/0.17-allow-new-players.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 47f9196..46e22a1 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -14,6 +14,11 @@ def should_allow(pr): (points_user, points_name, points_change) = bonuses[0] + pr_user = pr['user']['login'] + if pr_user != points_user: + raise Exception('New players should submit their own PRs, but %s submitted the PR to add %s' % + (pr_user, points_user)) + if points_name != 'initial': raise Exception('New player bonus value is called %s instead of "initial"' % points_name) @@ -29,9 +34,5 @@ def should_allow(pr): for bonus in os.listdir(bonus_directory): if bonus != 'initial': raise Exception('%s already has bonuses' % points_user) - - response = util.request('https://www.jefftk.com/nomic-github/users/%s' % points_user) - if response.status_code != 200: - raise Exception('%s is not a real GitHub user' % points_user) return True From 6ef4d818e97635b503b15ca7263f2cabe6832f37 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 14:04:28 -0500 Subject: [PATCH 07/15] Turns out there's an author already --- rules/0.17-allow-new-players.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 46e22a1..7dea4ce 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -14,10 +14,9 @@ def should_allow(pr): (points_user, points_name, points_change) = bonuses[0] - pr_user = pr['user']['login'] - if pr_user != points_user: + if pr.author != points_user: raise Exception('New players should submit their own PRs, but %s submitted the PR to add %s' % - (pr_user, points_user)) + (pr.author, points_user)) if points_name != 'initial': raise Exception('New player bonus value is called %s instead of "initial"' % points_name) From 46ffd59b6d2caead690c558475d7949562addf71 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Fri, 15 Feb 2019 14:06:14 -0500 Subject: [PATCH 08/15] But it's a method --- rules/0.17-allow-new-players.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 7dea4ce..35cb716 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -14,9 +14,9 @@ def should_allow(pr): (points_user, points_name, points_change) = bonuses[0] - if pr.author != points_user: + if pr.author() != points_user: raise Exception('New players should submit their own PRs, but %s submitted the PR to add %s' % - (pr.author, points_user)) + (pr.author(), points_user)) if points_name != 'initial': raise Exception('New player bonus value is called %s instead of "initial"' % points_name) From 3412590bb73e18dd8922d26f4374e8249d07a8d9 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Tue, 5 Mar 2019 10:58:06 -0500 Subject: [PATCH 09/15] Add some unit tests. Could use more. --- rules/0.25-block-test-failures.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rules/0.25-block-test-failures.py b/rules/0.25-block-test-failures.py index 500f98f..5b1f56b 100644 --- a/rules/0.25-block-test-failures.py +++ b/rules/0.25-block-test-failures.py @@ -2,6 +2,10 @@ import runpy SIMPLE_TESTS = { + '0.17-allow-new-players.py': [ + ('236', 'New players should submit their own PRs, but jeffkaufman submitted the PR to add sockpupper1'), + ('225', '10 initial points exceeds maximum starting value of 0 points'), + ], '0.3-allow-points-transfer.py': [ ('33', 'All file changes must be additions'), ('82', 'Points transfer PRs should not add users: got dchudz'), From 1835070295d9755d2043b5099c3956750015ce84 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Tue, 5 Mar 2019 11:18:30 -0500 Subject: [PATCH 10/15] More unit tests --- rules/0.25-block-test-failures.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rules/0.25-block-test-failures.py b/rules/0.25-block-test-failures.py index 5b1f56b..4d98d42 100644 --- a/rules/0.25-block-test-failures.py +++ b/rules/0.25-block-test-failures.py @@ -3,7 +3,14 @@ SIMPLE_TESTS = { '0.17-allow-new-players.py': [ + ('247', 'All file changes must be additions'), + ('250', 'Added file rules/0.22-block-nothing.py is not a bonus file'), + ('253', 'File should contain a single integer.'), + ('254', 'Only one new player can be added in a PR'), ('236', 'New players should submit their own PRs, but jeffkaufman submitted the PR to add sockpupper1'), + ('220', 'New player bonus value is called "lose-point" instead of "initial"'), + # TODO: Have a non-player submit a starting value of -1 + # ('?', 'Points cannot be negative'), ('225', '10 initial points exceeds maximum starting value of 0 points'), ], '0.3-allow-points-transfer.py': [ From 91d6fc23515be4394eb8ca2f2756c226bb4545be Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Tue, 5 Mar 2019 11:19:06 -0500 Subject: [PATCH 11/15] Quotes around incorrect file name --- rules/0.17-allow-new-players.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/0.17-allow-new-players.py b/rules/0.17-allow-new-players.py index 1a3dbac..db87807 100644 --- a/rules/0.17-allow-new-players.py +++ b/rules/0.17-allow-new-players.py @@ -20,7 +20,7 @@ def should_allow(pr): (pr.author(), points_user)) if points_name != 'initial': - raise Exception('New player bonus value is called %s instead of "initial"' % points_name) + raise Exception('New player bonus value is called "%s" instead of "initial"' % points_name) if points_change < 0: raise Exception('Points cannot be negative') From e33014c9990c12379e85c5cf44da8ecea96cbcb2 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Tue, 5 Mar 2019 11:20:31 -0500 Subject: [PATCH 12/15] Placeholder for valid test case --- rules/0.25-block-test-failures.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rules/0.25-block-test-failures.py b/rules/0.25-block-test-failures.py index 4d98d42..9faba85 100644 --- a/rules/0.25-block-test-failures.py +++ b/rules/0.25-block-test-failures.py @@ -12,6 +12,8 @@ # TODO: Have a non-player submit a starting value of -1 # ('?', 'Points cannot be negative'), ('225', '10 initial points exceeds maximum starting value of 0 points'), + # TODO: Have a non-player submit a starting value of 0 + # ('?', None), ], '0.3-allow-points-transfer.py': [ ('33', 'All file changes must be additions'), From 6236973b6f7615038e11d36cbedad873d8747b49 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Sun, 10 Mar 2019 19:56:40 -0400 Subject: [PATCH 13/15] 255 enabled a test --- rules/0.25-block-test-failures.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rules/0.25-block-test-failures.py b/rules/0.25-block-test-failures.py index 9faba85..6c3a451 100644 --- a/rules/0.25-block-test-failures.py +++ b/rules/0.25-block-test-failures.py @@ -12,8 +12,7 @@ # TODO: Have a non-player submit a starting value of -1 # ('?', 'Points cannot be negative'), ('225', '10 initial points exceeds maximum starting value of 0 points'), - # TODO: Have a non-player submit a starting value of 0 - # ('?', None), + ('255', None), ], '0.3-allow-points-transfer.py': [ ('33', 'All file changes must be additions'), From 6522b3d0322d7a30fc8e1a288c05b20c343c7442 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Sun, 10 Mar 2019 20:26:37 -0400 Subject: [PATCH 14/15] More test cases, thanks Dom! --- rules/0.25-block-test-failures.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rules/0.25-block-test-failures.py b/rules/0.25-block-test-failures.py index 6c3a451..ef6b356 100644 --- a/rules/0.25-block-test-failures.py +++ b/rules/0.25-block-test-failures.py @@ -9,10 +9,9 @@ ('254', 'Only one new player can be added in a PR'), ('236', 'New players should submit their own PRs, but jeffkaufman submitted the PR to add sockpupper1'), ('220', 'New player bonus value is called "lose-point" instead of "initial"'), - # TODO: Have a non-player submit a starting value of -1 - # ('?', 'Points cannot be negative'), + ('255', 'Points cannot be negative'), ('225', '10 initial points exceeds maximum starting value of 0 points'), - ('255', None), + ('258', None), ], '0.3-allow-points-transfer.py': [ ('33', 'All file changes must be additions'), From b19948158b6eb71b6015f0008ec2456802a647e1 Mon Sep 17 00:00:00 2001 From: Todd Nelling Date: Sun, 10 Mar 2019 21:02:29 -0400 Subject: [PATCH 15/15] More tests --- rules/0.25-block-test-failures.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rules/0.25-block-test-failures.py b/rules/0.25-block-test-failures.py index ef6b356..b41bdfe 100644 --- a/rules/0.25-block-test-failures.py +++ b/rules/0.25-block-test-failures.py @@ -4,7 +4,9 @@ SIMPLE_TESTS = { '0.17-allow-new-players.py': [ ('247', 'All file changes must be additions'), - ('250', 'Added file rules/0.22-block-nothing.py is not a bonus file'), + ('250', 'not enough values to unpack (expected 4, got 2)'), + ('260', 'Added file rules/tnelling/bonuses/initial is not a bonus file'), + ('261', 'Added file players/tnelling/foo/initial is not a bonus file'), ('253', 'File should contain a single integer.'), ('254', 'Only one new player can be added in a PR'), ('236', 'New players should submit their own PRs, but jeffkaufman submitted the PR to add sockpupper1'),