Skip to content

Commit e5bd806

Browse files
author
Yue Tu
committed
stop patching private methods
1 parent e8bede0 commit e5bd806

File tree

3 files changed

+40
-54
lines changed

3 files changed

+40
-54
lines changed

‎.python-version

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
3.6.8

‎tests/integ/test_git.py

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,16 @@ def test_git_support_with_pytorch(sagemaker_local_session):
3434
train_instance_count=1, train_instance_type='local',
3535
sagemaker_session=sagemaker_local_session, git_config=git_config)
3636

37-
train_input = pytorch.sagemaker_session.upload_data(path=os.path.join(data_path, 'training'),
38-
key_prefix='integ-test-data/pytorch_mnist/training')
39-
pytorch.fit({'training': train_input})
37+
pytorch.fit({'training': 'file://' + os.path.join(data_path, 'training')})
4038

41-
predictor = pytorch.deploy(initial_instance_count=1, instance_type='local')
39+
try:
40+
predictor = pytorch.deploy(initial_instance_count=1, instance_type='local')
4241

43-
data = numpy.zeros(shape=(1, 1, 28, 28)).astype(numpy.float32)
44-
result = predictor.predict(data)
45-
assert result is not None
42+
data = numpy.zeros(shape=(1, 1, 28, 28)).astype(numpy.float32)
43+
result = predictor.predict(data)
44+
assert result is not None
45+
finally:
46+
predictor.delete_endpoint()
4647

4748

4849
def test_git_support_with_mxnet(sagemaker_local_session, mxnet_full_version):
@@ -56,12 +57,19 @@ def test_git_support_with_mxnet(sagemaker_local_session, mxnet_full_version):
5657
train_instance_count=1, train_instance_type='local',
5758
sagemaker_session=sagemaker_local_session, git_config=git_config)
5859

59-
train_input = mx.sagemaker_session.upload_data(path=os.path.join(data_path, 'train'),
60-
key_prefix='integ-test-data/mxnet_mnist/train')
61-
test_input = mx.sagemaker_session.upload_data(path=os.path.join(data_path, 'test'),
62-
key_prefix='integ-test-data/mxnet_mnist/test')
63-
64-
mx.fit({'train': train_input, 'test': test_input})
60+
mx.fit({'train': 'file://' + os.path.join(data_path, 'train'),
61+
'test': 'file://' + os.path.join(data_path, 'test')})
6562

6663
files = [file for file in os.listdir(mx.source_dir)]
67-
assert 'some_file' in files and 'mnist.py' in files
64+
assert 'some_file' in files
65+
assert 'mnist.py' in files
66+
assert os.path.exists(mx.dependencies[0])
67+
68+
try:
69+
predictor = mx.deploy(initial_instance_count=1, instance_type='local')
70+
71+
data = numpy.zeros(shape=(1, 1, 28, 28))
72+
result = predictor.predict(data)
73+
assert result is not None
74+
finally:
75+
predictor.delete_endpoint()

‎tests/unit/test_git_utils.py

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,37 +26,30 @@
2626

2727
@patch('subprocess.check_call')
2828
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
29-
@patch('sagemaker.git_utils._validate_git_config')
30-
@patch('sagemaker.git_utils._checkout_branch_and_commit')
3129
@patch('os.path.isfile', return_value=True)
3230
@patch('os.path.isdir', return_value=True)
3331
@patch('os.path.exists', return_value=True)
34-
def test_git_clone_repo_succeed(exists, isdir, isfile, checkout_branch_and_commit,
35-
validate_git_config, mkdtemp, check_call):
32+
def test_git_clone_repo_succeed(exists, isdir, isfile, mkdtemp, check_call):
3633
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
3734
entry_point = 'entry_point'
3835
source_dir = 'source_dir'
3936
dependencies = ['foo', 'bar']
4037
ret = git_utils.git_clone_repo(git_config, entry_point, source_dir, dependencies)
41-
validate_git_config.assert_called_with(git_config)
42-
check_call.assert_called_with(['git', 'clone', git_config['repo'], REPO_DIR])
38+
check_call.assert_any_call(['git', 'clone', git_config['repo'], REPO_DIR])
39+
check_call.assert_any_call(args=['git', 'checkout', BRANCH], cwd=REPO_DIR)
40+
check_call.assert_any_call(args=['git', 'checkout', COMMIT], cwd=REPO_DIR)
4341
mkdtemp.assert_called_once()
44-
checkout_branch_and_commit.assert_called_with(git_config, REPO_DIR)
4542
assert ret['entry_point'] == 'entry_point'
4643
assert ret['source_dir'] == '/tmp/repo_dir/source_dir'
4744
assert ret['dependencies'] == ['/tmp/repo_dir/foo', '/tmp/repo_dir/bar']
4845

4946

5047
@patch('subprocess.check_call')
5148
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
52-
@patch('sagemaker.git_utils._validate_git_config',
53-
side_effect=ValueError('Please provide a repo for git_config.'))
54-
@patch('sagemaker.git_utils._checkout_branch_and_commit')
5549
@patch('os.path.isfile', return_value=True)
5650
@patch('os.path.isdir', return_value=True)
5751
@patch('os.path.exists', return_value=True)
58-
def test_git_clone_repo_repo_not_provided(exists, isdir, isfile, checkout_branch_and_commit,
59-
validate_git_config, mkdtemp, check_call):
52+
def test_git_clone_repo_repo_not_provided(exists, isdir, isfile, mkdtemp, check_call):
6053
git_config = {'branch': BRANCH, 'commit': COMMIT}
6154
entry_point = 'entry_point_that_does_not_exist'
6255
source_dir = 'source_dir'
@@ -69,13 +62,10 @@ def test_git_clone_repo_repo_not_provided(exists, isdir, isfile, checkout_branch
6962
@patch('subprocess.check_call',
7063
side_effect=subprocess.CalledProcessError(returncode=1, cmd='git clone {} {}'.format(GIT_REPO, REPO_DIR)))
7164
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
72-
@patch('sagemaker.git_utils._validate_git_config')
73-
@patch('sagemaker.git_utils._checkout_branch_and_commit')
7465
@patch('os.path.isfile', return_value=True)
7566
@patch('os.path.isdir', return_value=True)
7667
@patch('os.path.exists', return_value=True)
77-
def test_git_clone_repo_clone_fail(exists, isdir, isfile, checkout_branch_and_commit,
78-
validate_git_config, mkdtemp, check_call):
68+
def test_git_clone_repo_clone_fail(exists, isdir, isfile, mkdtemp, check_call):
7969
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
8070
entry_point = 'entry_point'
8171
source_dir = 'source_dir'
@@ -85,16 +75,14 @@ def test_git_clone_repo_clone_fail(exists, isdir, isfile, checkout_branch_and_co
8575
assert 'returned non-zero exit status' in str(error)
8676

8777

88-
@patch('subprocess.check_call')
78+
@patch('subprocess.check_call',
79+
side_effect=[True,
80+
subprocess.CalledProcessError(returncode=1, cmd='git checkout banana')])
8981
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
90-
@patch('sagemaker.git_utils._validate_git_config')
91-
@patch('sagemaker.git_utils._checkout_branch_and_commit',
92-
side_effect=subprocess.CalledProcessError(returncode=1, cmd='git checkout {}'.format(BRANCH)))
9382
@patch('os.path.isfile', return_value=True)
9483
@patch('os.path.isdir', return_value=True)
9584
@patch('os.path.exists', return_value=True)
96-
def test_git_clone_repo_branch_not_exist(exists, isdir, isfile, checkout_branch_and_commit,
97-
validate_git_config, mkdtemp, check_call):
85+
def test_git_clone_repo_branch_not_exist(exists, isdir, isfile, mkdtemp, check_call):
9886
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
9987
entry_point = 'entry_point'
10088
source_dir = 'source_dir'
@@ -104,16 +92,14 @@ def test_git_clone_repo_branch_not_exist(exists, isdir, isfile, checkout_branch_
10492
assert 'returned non-zero exit status' in str(error)
10593

10694

107-
@patch('subprocess.check_call')
95+
@patch('subprocess.check_call',
96+
side_effect=[True, True,
97+
subprocess.CalledProcessError(returncode=1, cmd='git checkout {}'.format(COMMIT))])
10898
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
109-
@patch('sagemaker.git_utils._validate_git_config')
110-
@patch('sagemaker.git_utils._checkout_branch_and_commit',
111-
side_effect=subprocess.CalledProcessError(returncode=1, cmd='git checkout {}'.format(COMMIT)))
11299
@patch('os.path.isfile', return_value=True)
113100
@patch('os.path.isdir', return_value=True)
114101
@patch('os.path.exists', return_value=True)
115-
def test_git_clone_repo_commit_not_exist(exists, isdir, isfile, checkout_branch_and_commit,
116-
validate_git_config, mkdtemp, check_call):
102+
def test_git_clone_repo_commit_not_exist(exists, isdir, isfile, mkdtemp, check_call):
117103
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
118104
entry_point = 'entry_point'
119105
source_dir = 'source_dir'
@@ -125,13 +111,10 @@ def test_git_clone_repo_commit_not_exist(exists, isdir, isfile, checkout_branch_
125111

126112
@patch('subprocess.check_call')
127113
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
128-
@patch('sagemaker.git_utils._validate_git_config')
129-
@patch('sagemaker.git_utils._checkout_branch_and_commit')
130114
@patch('os.path.isfile', return_value=False)
131115
@patch('os.path.isdir', return_value=True)
132116
@patch('os.path.exists', return_value=True)
133-
def test_git_clone_repo_entry_point_not_exist(exists, isdir, isfile, checkout_branch_and_commit,
134-
validate_git_config, mkdtemp, check_call):
117+
def test_git_clone_repo_entry_point_not_exist(exists, isdir, isfile, mkdtemp, check_call):
135118
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
136119
entry_point = 'entry_point_that_does_not_exist'
137120
source_dir = 'source_dir'
@@ -143,13 +126,10 @@ def test_git_clone_repo_entry_point_not_exist(exists, isdir, isfile, checkout_br
143126

144127
@patch('subprocess.check_call')
145128
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
146-
@patch('sagemaker.git_utils._validate_git_config')
147-
@patch('sagemaker.git_utils._checkout_branch_and_commit')
148129
@patch('os.path.isfile', return_value=True)
149130
@patch('os.path.isdir', return_value=False)
150131
@patch('os.path.exists', return_value=True)
151-
def test_git_clone_repo_source_dir_not_exist(exists, isdir, isfile, checkout_branch_and_commit,
152-
validate_git_config, mkdtemp, check_call):
132+
def test_git_clone_repo_source_dir_not_exist(exists, isdir, isfile, mkdtemp, check_call):
153133
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
154134
entry_point = 'entry_point'
155135
source_dir = 'source_dir_that_does_not_exist'
@@ -161,13 +141,10 @@ def test_git_clone_repo_source_dir_not_exist(exists, isdir, isfile, checkout_bra
161141

162142
@patch('subprocess.check_call')
163143
@patch('tempfile.mkdtemp', return_value=REPO_DIR)
164-
@patch('sagemaker.git_utils._validate_git_config')
165-
@patch('sagemaker.git_utils._checkout_branch_and_commit')
166144
@patch('os.path.isfile', return_value=True)
167145
@patch('os.path.isdir', return_value=True)
168146
@patch('os.path.exists', side_effect=[True, False])
169-
def test_git_clone_repo_dependencies_not_exist(exists, isdir, isfile, checkout_branch_and_commit,
170-
validate_git_config, mkdtemp, check_call):
147+
def test_git_clone_repo_dependencies_not_exist(exists, isdir, isfile, mkdtemp, check_call):
171148
git_config = {'repo': GIT_REPO, 'branch': BRANCH, 'commit': COMMIT}
172149
entry_point = 'entry_point'
173150
source_dir = 'source_dir'

0 commit comments

Comments
 (0)