From 9e2edb910a28458dd88b2c3a7b1241f60dff320f Mon Sep 17 00:00:00 2001 From: Mukundan Sundararajan Date: Sun, 24 Dec 2017 15:03:09 -0800 Subject: [PATCH] Fixed run argument check for mounting volumes. Refactored argument validation test. --- repo2docker/app.py | 3 +- tests/argumentvalidation.py | 73 ++++++++++++++++++++++++++----------- 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index c44a9b6d..2ac995ee 100644 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -369,7 +369,8 @@ class Repo2Docker(Application): self.run = False self.push = False - if args.volumes and not args.run: + # check against self.run and not args.run as self.run is false on --no-build + if args.volumes and not self.run: # Can't mount if we aren't running print("To Mount volumes with -v, you also need to run the container") sys.exit(1) diff --git a/tests/argumentvalidation.py b/tests/argumentvalidation.py index eb895081..f059cd16 100644 --- a/tests/argumentvalidation.py +++ b/tests/argumentvalidation.py @@ -5,24 +5,17 @@ Tests that runs validity checks on arguments passed in from shell import os import subprocess -def does_validate_image_name(builddir, image_name): +def validate_arguments(builddir, args_list, expected): try: - output = subprocess.check_output( - [ - 'repo2docker', - '--no-run', - '--no-build', - '--image-name', - str(image_name), - builddir - ], - stderr=subprocess.STDOUT, - ).decode() + cmd = ['repo2docker'] + for k in args_list: + cmd.append(k) + cmd.append(builddir) + subprocess.check_output(cmd, stderr=subprocess.STDOUT) return True except subprocess.CalledProcessError as e: output = e.output.decode() - if "error: argument --image-name: %r is not a valid docker image name. " \ - "Image name can contain only lowercase characters." % image_name in output: + if expected in output: return False else: raise @@ -35,8 +28,11 @@ def test_image_name_fail(): """ builddir = os.path.dirname(__file__) - - assert not does_validate_image_name(builddir, 'Test/Invalid_name:1.0.0') + image_name = 'Test/Invalid_name:1.0.0' + args_list = ['--no-run', '--no-build', '--image-name', image_name] + expected = "error: argument --image-name: %r is not a valid docker image name. " \ + "Image name can contain only lowercase characters." % image_name + assert not validate_arguments(builddir, args_list, expected) def test_image_name_underscore_fail(): @@ -45,8 +41,12 @@ def test_image_name_underscore_fail(): """ builddir = os.path.dirname(__file__) + image_name = '_test/invalid_name:1.0.0' + args_list = ['--no-run', '--no-build', '--image-name', image_name] + expected = "error: argument --image-name: %r is not a valid docker image name. " \ + "Image name can contain only lowercase characters." % image_name - assert not does_validate_image_name(builddir, '_test/invalid_name:1.0.0') + assert not validate_arguments(builddir, args_list, expected) def test_image_name_double_dot_fail(): @@ -55,8 +55,12 @@ def test_image_name_double_dot_fail(): """ builddir = os.path.dirname(__file__) + image_name = 'test..com/invalid_name:1.0.0' + args_list = ['--no-run', '--no-build', '--image-name', image_name] + expected = "error: argument --image-name: %r is not a valid docker image name. " \ + "Image name can contain only lowercase characters." % image_name - assert not does_validate_image_name(builddir, 'test..com/invalid_name:1.0.0') + assert not validate_arguments(builddir, args_list, expected) def test_image_name_valid_restircted_registry_domain_name_fail(): @@ -66,8 +70,12 @@ def test_image_name_valid_restircted_registry_domain_name_fail(): """ builddir = os.path.dirname(__file__) + image_name = 'Test.com/valid_name:1.0.0' + args_list = ['--no-run', '--no-build', '--image-name', image_name] + expected = "error: argument --image-name: %r is not a valid docker image name. " \ + "Image name can contain only lowercase characters." % image_name - assert not does_validate_image_name(builddir, 'Test.com/valid_name:1.0.0') + assert not validate_arguments(builddir, args_list, expected) def test_image_name_valid_registry_domain_name_success(): @@ -76,8 +84,10 @@ def test_image_name_valid_registry_domain_name_success(): """ builddir = os.path.dirname(__file__) + '/dockerfile/simple/' + image_name = 'test.COM/valid_name:1.0.0' + args_list = ['--no-run', '--no-build', '--image-name', image_name] - assert does_validate_image_name(builddir, 'test.COM/valid_name:1.0.0') + assert validate_arguments(builddir, args_list, None) def test_image_name_valid_name_success(): @@ -86,5 +96,26 @@ def test_image_name_valid_name_success(): """ builddir = os.path.dirname(__file__) + '/dockerfile/simple/' + image_name = 'test.com/valid_name:1.0.0' + args_list = ['--no-run', '--no-build', '--image-name', image_name] - assert does_validate_image_name(builddir, 'test.com/valid_name:1.0.0') \ No newline at end of file + assert validate_arguments(builddir, args_list, None) + +def test_volume_no_build_fail(): + """ + Test to check if repo2docker fails when both --no-build and -v arguments are given + """ + builddir = os.path.dirname(__file__) + args_list = ['--no-build', '-v', '/data:/data'] + + assert not validate_arguments(builddir, args_list, 'To Mount volumes with -v, you also need to run the container') + + +def test_volume_no_run_fail(): + """ + Test to check if repo2docker fails when both --no-run and -v arguments are given + """ + builddir = os.path.dirname(__file__) + args_list = ['--no-run', '-v', '/data:/data'] + + assert not validate_arguments(builddir, args_list, 'To Mount volumes with -v, you also need to run the container') \ No newline at end of file