Skip to content

Commit 4b84dc6

Browse files
Neeratyoymfeurer
authored andcommitted
Making unit tests green (#748)
* Adding flaky rerun decorators for stochastic failures * Increasing number of repeats for stochastic failures * Increasing retries; Fixing PEP8 * Small update to logging behaviour for unit testing * Increasing retries till it works * Fixing unit test waiting for server processing * Revamping deletion of files after unit tests * Adding comments/descriptions * Debugging * Debugging * Fixing directory issue for test cases * Doubling wait time for test_run_and_upload_gridsearch * Removing semaphore implementation * Fixing path issue for appveyor tests * Debugging appveyor path * Fixing PEP8 * Fixing test_list_datasets_with_high_size_parameter * PEP8 fix * Removing logging to disk
1 parent 053623d commit 4b84dc6

File tree

6 files changed

+204
-131
lines changed

6 files changed

+204
-131
lines changed

ci_scripts/test.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ cd $curr_dir
4545
# compares with $before to check for remaining files
4646
after="`git status --porcelain -b`"
4747
if [[ "$before" != "$after" ]]; then
48+
echo 'git status from before: '$before
49+
echo 'git status from after: '$after
4850
echo "All generated files have not been deleted!"
4951
exit 1
50-
fi
52+
fi

openml/testing.py

Lines changed: 3 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import openml
1818
from openml.tasks import TaskTypeEnum
1919

20-
import pytest
2120
import logging
2221

2322

@@ -35,12 +34,9 @@ class TestBase(unittest.TestCase):
3534
# amueller's read/write key that he will throw away later
3635
apikey = "610344db6388d9ba34f6db45a3cf71de"
3736

38-
# creating logger for unit test file deletion status
39-
logger = logging.getLogger("unit_tests")
40-
logger.setLevel(logging.INFO)
41-
fh = logging.FileHandler('TestBase.log')
42-
fh.setLevel(logging.INFO)
43-
logger.addHandler(fh)
37+
# creating logger for tracking files uploaded to test server
38+
logger = logging.getLogger("unit_tests_published_entities")
39+
logger.setLevel(logging.DEBUG)
4440

4541
def setUp(self, n_levels: int = 1):
4642
"""Setup variables and temporary directories.
@@ -151,82 +147,6 @@ def _delete_entity_from_tracker(self, entity_type, entity):
151147
if id_ == entity][0]
152148
TestBase.publish_tracker[entity_type].pop(delete_index)
153149

154-
@pytest.fixture(scope="session", autouse=True)
155-
def _cleanup_fixture(self):
156-
"""Cleans up files generated by unit tests
157-
158-
This function is called at the beginning of the invocation of
159-
TestBase (defined below), by each of class that inherits TestBase.
160-
The 'yield' creates a checkpoint and breaks away to continue running
161-
the unit tests of the sub class. When all the tests end, execution
162-
resumes from the checkpoint.
163-
"""
164-
165-
abspath_this_file = os.path.abspath(inspect.getfile(self.__class__))
166-
static_cache_dir = os.path.dirname(abspath_this_file)
167-
# Could be a risky while condition, however, going up a directory
168-
# n-times will eventually end at main directory
169-
while True:
170-
if 'openml' in os.listdir(static_cache_dir):
171-
break
172-
else:
173-
static_cache_dir = os.path.join(static_cache_dir, '../')
174-
directory = os.path.join(static_cache_dir, 'tests/files/')
175-
files = os.walk(directory)
176-
old_file_list = []
177-
for root, _, filenames in files:
178-
for filename in filenames:
179-
old_file_list.append(os.path.join(root, filename))
180-
# context switches to other remaining tests
181-
# pauses the code execution here till all tests in the 'session' is over
182-
yield
183-
# resumes from here after all collected tests are completed
184-
185-
#
186-
# Local file deletion
187-
#
188-
files = os.walk(directory)
189-
new_file_list = []
190-
for root, _, filenames in files:
191-
for filename in filenames:
192-
new_file_list.append(os.path.join(root, filename))
193-
# filtering the files generated during this run
194-
new_file_list = list(set(new_file_list) - set(old_file_list))
195-
for file in new_file_list:
196-
os.remove(file)
197-
198-
#
199-
# Test server deletion
200-
#
201-
openml.config.server = TestBase.test_server
202-
openml.config.apikey = TestBase.apikey
203-
204-
# legal_entities defined in openml.utils._delete_entity - {'user'}
205-
entity_types = {'run', 'data', 'flow', 'task', 'study'}
206-
# 'run' needs to be first entity to allow other dependent entities to be deleted
207-
# cloning file tracker to allow deletion of entries of deleted files
208-
tracker = TestBase.publish_tracker.copy()
209-
210-
# reordering to delete sub flows at the end of flows
211-
# sub-flows have shorter names, hence, sorting by descending order of flow name length
212-
if 'flow' in entity_types:
213-
flow_deletion_order = [entity_id for entity_id, _ in
214-
sorted(tracker['flow'], key=lambda x: len(x[1]), reverse=True)]
215-
tracker['flow'] = flow_deletion_order
216-
217-
# deleting all collected entities published to test server
218-
for entity_type in entity_types:
219-
for i, entity in enumerate(tracker[entity_type]):
220-
try:
221-
openml.utils._delete_entity(entity_type, entity)
222-
TestBase.logger.info("Deleted ({}, {})".format(entity_type, entity))
223-
# deleting actual entry from tracker
224-
TestBase._delete_entity_from_tracker(entity_type, entity)
225-
except Exception as e:
226-
TestBase.logger.warning("Cannot delete ({},{}): {}".format(
227-
entity_type, entity, e))
228-
TestBase.logger.info("End of cleanup_fixture from {}".format(self.__class__))
229-
230150
def _get_sentinel(self, sentinel=None):
231151
if sentinel is None:
232152
# Create a unique prefix for the flow. Necessary because the flow

tests/conftest.py

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
'''This file is recognized by pytest for defining specified behaviour
2+
3+
'conftest.py' files are directory-scope files that are shared by all
4+
sub-directories from where this file is placed. pytest recognises
5+
'conftest.py' for any unit test executed from within this directory
6+
tree. This file is used to define fixtures, hooks, plugins, and other
7+
functionality that can be shared by the unit tests.
8+
9+
This file has been created for the OpenML testing to primarily make use
10+
of the pytest hooks 'pytest_sessionstart' and 'pytest_sessionfinish',
11+
which are being used for managing the deletion of local and remote files
12+
created by the unit tests, run across more than one process.
13+
14+
This design allows one to comment or remove the conftest.py file to
15+
disable file deletions, without editing any of the test case files.
16+
17+
18+
Possible Future: class TestBase from openml/testing.py can be included
19+
under this file and there would not be any requirements to import
20+
testing.py in each of the unit test modules.
21+
'''
22+
23+
import os
24+
import logging
25+
from typing import List
26+
27+
import openml
28+
from openml.testing import TestBase
29+
30+
# creating logger for unit test file deletion status
31+
logger = logging.getLogger("unit_tests")
32+
logger.setLevel(logging.DEBUG)
33+
34+
file_list = []
35+
directory = None
36+
37+
# finding the root directory of conftest.py and going up to OpenML main directory
38+
# exploiting the fact that conftest.py always resides in the root directory for tests
39+
static_dir = os.path.dirname(os.path.abspath(__file__))
40+
logging.info("static directory: {}".format(static_dir))
41+
print("static directory: {}".format(static_dir))
42+
while True:
43+
if 'openml' in os.listdir(static_dir):
44+
break
45+
static_dir = os.path.join(static_dir, '..')
46+
47+
48+
def worker_id() -> str:
49+
''' Returns the name of the worker process owning this function call.
50+
51+
:return: str
52+
Possible outputs from the set of {'master', 'gw0', 'gw1', ..., 'gw(n-1)'}
53+
where n is the number of workers being used by pytest-xdist
54+
'''
55+
vars_ = list(os.environ.keys())
56+
if 'PYTEST_XDIST_WORKER' in vars_ or 'PYTEST_XDIST_WORKER_COUNT' in vars_:
57+
return os.environ['PYTEST_XDIST_WORKER']
58+
else:
59+
return 'master'
60+
61+
62+
def read_file_list() -> List[str]:
63+
'''Returns a list of paths to all files that currently exist in 'openml/tests/files/'
64+
65+
:return: List[str]
66+
'''
67+
directory = os.path.join(static_dir, 'tests/files/')
68+
if worker_id() == 'master':
69+
logger.info("Collecting file lists from: {}".format(directory))
70+
files = os.walk(directory)
71+
file_list = []
72+
for root, _, filenames in files:
73+
for filename in filenames:
74+
file_list.append(os.path.join(root, filename))
75+
return file_list
76+
77+
78+
def compare_delete_files(old_list, new_list) -> None:
79+
'''Deletes files that are there in the new_list but not in the old_list
80+
81+
:param old_list: List[str]
82+
:param new_list: List[str]
83+
:return: None
84+
'''
85+
file_list = list(set(new_list) - set(old_list))
86+
for file in file_list:
87+
os.remove(file)
88+
logger.info("Deleted from local: {}".format(file))
89+
90+
91+
def delete_remote_files(tracker) -> None:
92+
'''Function that deletes the entities passed as input, from the OpenML test server
93+
94+
The TestBase class in openml/testing.py has an attribute called publish_tracker.
95+
This function expects the dictionary of the same structure.
96+
It is a dictionary of lists, where the keys are entity types, while the values are
97+
lists of integer IDs, except for key 'flow' where the value is a tuple (ID, flow name).
98+
99+
Iteratively, multiple POST requests are made to the OpenML test server using
100+
openml.utils._delete_entity() to remove the entities uploaded by all the unit tests.
101+
102+
:param tracker: Dict
103+
:return: None
104+
'''
105+
openml.config.server = TestBase.test_server
106+
openml.config.apikey = TestBase.apikey
107+
108+
# reordering to delete sub flows at the end of flows
109+
# sub-flows have shorter names, hence, sorting by descending order of flow name length
110+
if 'flow' in tracker:
111+
flow_deletion_order = [entity_id for entity_id, _ in
112+
sorted(tracker['flow'], key=lambda x: len(x[1]), reverse=True)]
113+
tracker['flow'] = flow_deletion_order
114+
115+
# deleting all collected entities published to test server
116+
# 'run's are deleted first to prevent dependency issue of entities on deletion
117+
logger.info("Entity Types: {}".format(['run', 'data', 'flow', 'task', 'study']))
118+
for entity_type in ['run', 'data', 'flow', 'task', 'study']:
119+
logger.info("Deleting {}s...".format(entity_type))
120+
for i, entity in enumerate(tracker[entity_type]):
121+
try:
122+
openml.utils._delete_entity(entity_type, entity)
123+
logger.info("Deleted ({}, {})".format(entity_type, entity))
124+
except Exception as e:
125+
logger.warn("Cannot delete ({},{}): {}".format(entity_type, entity, e))
126+
127+
128+
def pytest_sessionstart() -> None:
129+
'''pytest hook that is executed before any unit test starts
130+
131+
This function will be called by each of the worker processes, along with the master process
132+
when they are spawned. This happens even before the collection of unit tests.
133+
If number of workers, n=4, there will be a total of 5 (1 master + 4 workers) calls of this
134+
function, before execution of any unit test begins. The master pytest process has the name
135+
'master' while the worker processes are named as 'gw{i}' where i = 0, 1, ..., n-1.
136+
The order of process spawning is: 'master' -> random ordering of the 'gw{i}' workers.
137+
138+
Since, master is always executed first, it is checked if the current process is 'master' and
139+
store a list of strings of paths of all files in the directory (pre-unit test snapshot).
140+
141+
:return: None
142+
'''
143+
# file_list is global to maintain the directory snapshot during tear down
144+
global file_list
145+
worker = worker_id()
146+
if worker == 'master':
147+
file_list = read_file_list()
148+
149+
150+
def pytest_sessionfinish() -> None:
151+
'''pytest hook that is executed after all unit tests of a worker ends
152+
153+
This function will be called by each of the worker processes, along with the master process
154+
when they are done with the unit tests allocated to them.
155+
If number of workers, n=4, there will be a total of 5 (1 master + 4 workers) calls of this
156+
function, before execution of any unit test begins. The master pytest process has the name
157+
'master' while the worker processes are named as 'gw{i}' where i = 0, 1, ..., n-1.
158+
The order of invocation is: random ordering of the 'gw{i}' workers -> 'master'.
159+
160+
Since, master is always executed last, it is checked if the current process is 'master' and,
161+
* Compares file list with pre-unit test snapshot and deletes all local files generated
162+
* Iterates over the list of entities uploaded to test server and deletes them remotely
163+
164+
:return: None
165+
'''
166+
# allows access to the file_list read in the set up phase
167+
global file_list
168+
worker = worker_id()
169+
logger.info("Finishing worker {}".format(worker))
170+
171+
# Test file deletion
172+
logger.info("Deleting files uploaded to test server for worker {}".format(worker))
173+
delete_remote_files(TestBase.publish_tracker)
174+
175+
if worker == 'master':
176+
# Local file deletion
177+
new_file_list = read_file_list()
178+
compare_delete_files(file_list, new_file_list)
179+
logger.info("Local files deleted")
180+
181+
logging.info("{} is killed".format(worker))

tests/test_datasets/test_dataset_functions.py

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from unittest import mock
55

66
import arff
7+
import time
78

89
import pytest
910
import numpy as np
@@ -1088,22 +1089,8 @@ def test_ignore_attributes_dataset(self):
10881089
paper_url=paper_url
10891090
)
10901091

1091-
def test___publish_fetch_ignore_attribute(self):
1092-
"""(Part 1) Test to upload and retrieve dataset and check ignore_attributes
1093-
1094-
DEPENDS on test_publish_fetch_ignore_attribute() to be executed after this
1095-
This test is split into two parts:
1096-
1) test___publish_fetch_ignore_attribute()
1097-
This will be executed earlier, owing to alphabetical sorting.
1098-
This test creates and publish() a dataset and checks for a valid ID.
1099-
2) test_publish_fetch_ignore_attribute()
1100-
This will be executed after test___publish_fetch_ignore_attribute(),
1101-
owing to alphabetical sorting. The time gap is to allow the server
1102-
more time time to compute data qualities.
1103-
The dataset ID obtained previously is used to fetch the dataset.
1104-
The retrieved dataset is checked for valid ignore_attributes.
1105-
"""
1106-
# the returned fixt
1092+
def test_publish_fetch_ignore_attribute(self):
1093+
"""Test to upload and retrieve dataset and check ignore_attributes"""
11071094
data = [
11081095
['a', 'sunny', 85.0, 85.0, 'FALSE', 'no'],
11091096
['b', 'sunny', 80.0, 90.0, 'TRUE', 'no'],
@@ -1158,40 +1145,22 @@ def test___publish_fetch_ignore_attribute(self):
11581145
upload_did))
11591146
# test if publish was successful
11601147
self.assertIsInstance(upload_did, int)
1161-
# variables to carry forward for test_publish_fetch_ignore_attribute()
1162-
self.__class__.test_publish_fetch_ignore_attribute_did = upload_did
1163-
self.__class__.test_publish_fetch_ignore_attribute_list = ignore_attribute
11641148

1165-
def test_publish_fetch_ignore_attribute(self):
1166-
"""(Part 2) Test to upload and retrieve dataset and check ignore_attributes
1167-
1168-
DEPENDS on test___publish_fetch_ignore_attribute() to be executed first
1169-
This will be executed after test___publish_fetch_ignore_attribute(),
1170-
owing to alphabetical sorting. The time gap is to allow the server
1171-
more time time to compute data qualities.
1172-
The dataset ID obtained previously is used to fetch the dataset.
1173-
The retrieved dataset is checked for valid ignore_attributes.
1174-
"""
1175-
# Retrieving variables from test___publish_fetch_ignore_attribute()
1176-
upload_did = self.__class__.test_publish_fetch_ignore_attribute_did
1177-
ignore_attribute = self.__class__.test_publish_fetch_ignore_attribute_list
1178-
trials = 1
1179-
timeout_limit = 200
11801149
dataset = None
11811150
# fetching from server
11821151
# loop till timeout or fetch not successful
1183-
while True:
1184-
if trials > timeout_limit:
1185-
break
1152+
max_waiting_time_seconds = 400
1153+
# time.time() works in seconds
1154+
start_time = time.time()
1155+
while time.time() - start_time < max_waiting_time_seconds:
11861156
try:
11871157
dataset = openml.datasets.get_dataset(upload_did)
11881158
break
11891159
except Exception as e:
11901160
# returned code 273: Dataset not processed yet
11911161
# returned code 362: No qualities found
1192-
print("Trial {}/{}: ".format(trials, timeout_limit))
1193-
print("\tFailed to fetch dataset:{} with '{}'.".format(upload_did, str(e)))
1194-
trials += 1
1162+
print("Failed to fetch dataset:{} with '{}'.".format(upload_did, str(e)))
1163+
time.sleep(10)
11951164
continue
11961165
if dataset is None:
11971166
raise ValueError("TIMEOUT: Failed to fetch uploaded dataset - {}".format(upload_did))

tests/test_runs/test_run_functions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ def determine_grid_size(param_grid):
411411
# suboptimal (slow), and not guaranteed to work if evaluation
412412
# engine is behind.
413413
# TODO: mock this? We have the arff already on the server
414-
self._wait_for_processed_run(run.run_id, 200)
414+
self._wait_for_processed_run(run.run_id, 400)
415415
try:
416416
model_prime = openml.runs.initialize_model_from_trace(
417417
run_id=run.run_id,

0 commit comments

Comments
 (0)