Skip to content

Fix flaky split test #929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kohsuke merged 6 commits into from
Nov 6, 2024
Merged

Fix flaky split test #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
570eb9e
add comments
Konboi Nov 6, 2024
147330a
refactor tests
Konboi Nov 6, 2024
266e3c1
for debug
Konboi Nov 6, 2024
1b821d4
add prefix to avoid conflict
Konboi Nov 6, 2024
20dd6aa
Don't know the cause, but in the Python 3.10 environment, we need to …
Konboi Nov 6, 2024
c92a645
Removing this as instructed by Konboi
kohsuke Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions tests/cli_test_case.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,13 @@ def assert_success(self, result: click.testing.Result):
def assert_exit_code(self, result: click.testing.Result, expected: int):
self.assertEqual(result.exit_code, expected, result.stdout)

def assert_contents(self, file_path: str, content: str):
with open(file_path) as f:
self.assertEqual(f.read(), content)

def assert_file_exists(self, file_path: str, exists: bool = True):
self.assertEqual(Path(file_path).exists(), exists)

def find_request(self, url_suffix: str, n: int = 0):
'''Find the first (or n-th, if specified) request that matches the given suffix'''
for call in responses.calls:
Expand Down
125 changes: 55 additions & 70 deletions tests/commands/test_split_subset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import pprint
import tempfile
from unittest import mock

Expand Down Expand Up @@ -146,77 +147,65 @@ def test_split_by_group_names(self):
]
}

responses.replace(
responses.POST,
"{base_url}/intake/organizations/{organization}/workspaces/{workspace}/subset/{subset_id}/split-by-groups".format(
base_url=get_base_url(),
organization=self.organization,
workspace=self.workspace,
subset_id=self.subsetting_id,
),
json=mock_json_response,
status=200
)
"""
Note(Konboi):
Don't know the cause, but in the Python 3.10 environment,
the settings configured with responses.replace disappear on the second call.
see: https://github.com/launchableinc/cli/actions/runs/11697720998/job/32576899978#step:10:88
So, to call it each time, `replace_response` was defined.
"""
def replace_response():
responses.replace(
responses.POST,
"{base_url}/intake/organizations/{organization}/workspaces/{workspace}/subset/{subset_id}/split-by-groups".format(
base_url=get_base_url(),
organization=self.organization,
workspace=self.workspace,
subset_id=self.subsetting_id,
),
json=mock_json_response,
status=200
)

with tempfile.TemporaryDirectory() as tmpdir:
replace_response()
result = self.cli("split-subset", "--subset-id", "subset/{}".format(self.subsetting_id),
"--split-by-groups", "--split-by-groups-output-dir", tmpdir, "file")

self.assert_success(result)
with open("{}/subset-e2e.txt".format(tmpdir)) as f:
self.assertEqual(f.read(), "e2e-aaa.py\ne2e-bbb.py")

self.assertFalse(os.path.exists("{}/rest-e2e.txt".format(tmpdir)))
self.assert_contents("{}/subset-e2e.txt".format(tmpdir), "e2e-aaa.py\ne2e-bbb.py")
self.assert_contents("{}/subset-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME), "aaa.py\nbbb.py")
# check the group file
self.assert_contents("{}/{}".format(tmpdir, SPLIT_BY_GROUP_SUBSET_GROUPS_FILE_NAME), "e2e")

self.assertFalse(os.path.exists("{}/subset-unit-test.txt".format(tmpdir)))
# server doesn't return subset of unit-test
self.assert_file_exists("{}/subset-unit-test.txt".format(tmpdir), False)

self.assertFalse(os.path.exists("{}/rest-unit-test.txt".format(tmpdir)))

with open("{}/subset-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME)) as f:
self.assertEqual(f.read(), "aaa.py\nbbb.py")

self.assertFalse(os.path.exists("{}/rest-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME)))

with open("{}/{}".format(tmpdir, SPLIT_BY_GROUP_SUBSET_GROUPS_FILE_NAME)) as f:
self.assertEqual(f.read(), "e2e")

self.assertFalse(os.path.exists("{}/{}".format(tmpdir, SPLIT_BY_GROUP_REST_GROUPS_FILE_NAME)))
# doesn't set the --rest option
self.assert_file_exists("{}/rest-e2e.txt".format(tmpdir), False)
self.assert_file_exists("{}/rest-unit-test.txt".format(tmpdir), False)
self.assert_file_exists("{}/rest-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME), False)
self.assert_file_exists("{}/{}".format(tmpdir, SPLIT_BY_GROUP_REST_GROUPS_FILE_NAME), False)

# with rest option
with tempfile.TemporaryDirectory() as tmpdir:
replace_response()
result = self.cli("split-subset", "--subset-id", "subset/{}".format(self.subsetting_id),
"--split-by-groups-with-rest", "--split-by-groups-output-dir", tmpdir, "file")
"--split-by-groups-with-rest", "--split-by-groups-output-dir", tmpdir, "file", mix_stderr=False)

self.assert_success(result)
# this test is flaky -- let's see what's going on
print("println debugging of a flaky test---")
print(result.stdout)
for item in os.listdir(tmpdir):
print(item)
print("--- till here")

with open(os.path.join(tmpdir, "subset-e2e.txt")) as f:
self.assertEqual(f.read(), "e2e-aaa.py\ne2e-bbb.py")

with open(os.path.join(tmpdir, "rest-e2e.txt")) as f:
self.assertEqual(f.read(), "e2e-ccc.py\ne2e-ddd.py")
self.assert_contents("{}/subset-e2e.txt".format(tmpdir), "e2e-aaa.py\ne2e-bbb.py")
self.assert_contents("{}/rest-e2e.txt".format(tmpdir), "e2e-ccc.py\ne2e-ddd.py")

self.assertFalse(os.path.exists("{}/subset-unit-test.txt".format(tmpdir)))
# server doesn't return subset of unit-test
self.assert_file_exists("{}/subset-unit-test.txt".format(tmpdir), False)
self.assert_contents("{}/rest-unit-test.txt".format(tmpdir), "unit-test-111.py\nunit-test-222.py")

with open(os.path.join(tmpdir, "rest-unit-test.txt")) as f:
self.assertEqual(f.read(), "unit-test-111.py\nunit-test-222.py")
self.assert_contents("{}/subset-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME), "aaa.py\nbbb.py")
self.assert_contents("{}/rest-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME), "111.py\n222.py")

with open(os.path.join(tmpdir, "subset-{}.txt".format(SPLIT_BY_GROUPS_NO_GROUP_NAME))) as f:
self.assertEqual(f.read(), "aaa.py\nbbb.py")

with open(os.path.join(tmpdir, "rest-{}.txt".format(SPLIT_BY_GROUPS_NO_GROUP_NAME))) as f:
self.assertEqual(f.read(), "111.py\n222.py")

with open(os.path.join(tmpdir, SPLIT_BY_GROUP_SUBSET_GROUPS_FILE_NAME)) as f:
self.assertEqual(f.read(), "e2e")

with open(os.path.join(tmpdir, SPLIT_BY_GROUP_REST_GROUPS_FILE_NAME)) as f:
self.assertEqual(f.read(), "unit-test")
# check the group file
self.assert_contents("{}/{}".format(tmpdir, SPLIT_BY_GROUP_SUBSET_GROUPS_FILE_NAME), "e2e")
self.assert_contents("{}/{}".format(tmpdir, SPLIT_BY_GROUP_REST_GROUPS_FILE_NAME), "unit-test")

@responses.activate
@mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token})
Expand Down Expand Up @@ -275,19 +264,15 @@ def test_split_by_group_names_output_exclusion_rules(self):
"--split-by-groups", "--split-by-groups-output-dir", tmpdir, '--output-exclusion-rules', "file")

self.assert_success(result)
with open("{}/subset-e2e.txt".format(tmpdir)) as f:
self.assertCountEqual(f.read().splitlines(), ["e2e-ccc.py", "e2e-ddd.py", ])

self.assertFalse(os.path.exists("{}/rest-e2e.txt".format(tmpdir)))

with open("{}/subset-unit-test.txt".format(tmpdir)) as f:
self.assertCountEqual(f.read().splitlines(), ["unit-test-111.py", "unit-test-222.py"])
self.assertFalse(os.path.exists("{}/rest-unit-test.txt".format(tmpdir)))

with open("{}/subset-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME)) as f:
self.assertCountEqual(f.read().splitlines(), ["111.py", "222.py"])
self.assertFalse(os.path.exists("{}/rest-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME)))

with open("{}/{}".format(tmpdir, SPLIT_BY_GROUP_SUBSET_GROUPS_FILE_NAME)) as f:
self.assertEqual(f.read(), "unit-test")
self.assertFalse(os.path.exists("{}/{}".format(tmpdir, SPLIT_BY_GROUP_REST_GROUPS_FILE_NAME)))
# --output-exclusion-rules is enabled, thus switched subset and rest
self.assert_contents("{}/subset-e2e.txt".format(tmpdir), "e2e-ccc.py\ne2e-ddd.py")
self.assert_contents("{}/subset-unit-test.txt".format(tmpdir), "unit-test-111.py\nunit-test-222.py")
self.assert_contents("{}/subset-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME), "111.py\n222.py")
self.assert_contents("{}/{}".format(tmpdir, SPLIT_BY_GROUP_SUBSET_GROUPS_FILE_NAME), "unit-test")

# doesn't set the --rest option
self.assert_file_exists("{}/rest-e2e.txt".format(tmpdir), False)
self.assert_file_exists("{}/rest-unit-test.txt".format(tmpdir), False)
self.assert_file_exists("{}/rest-{}.txt".format(tmpdir, SPLIT_BY_GROUPS_NO_GROUP_NAME), False)
self.assert_file_exists("{}/{}".format(tmpdir, SPLIT_BY_GROUP_REST_GROUPS_FILE_NAME), False)