From df68855147c8bdc6b863ef9e4daec52a3c8cfe52 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Thu, 8 Oct 2020 21:23:50 +0000 Subject: workspace.py: Simplify test by only allowing opening a single workspace This test seems to be copy pasted from our tests around workspaces and doesn't need to be so complicated --- src/buildstream/testing/_sourcetests/workspace.py | 62 ++++------------------- 1 file changed, 10 insertions(+), 52 deletions(-) diff --git a/src/buildstream/testing/_sourcetests/workspace.py b/src/buildstream/testing/_sourcetests/workspace.py index 3520a8cc4..7f3c5dea4 100644 --- a/src/buildstream/testing/_sourcetests/workspace.py +++ b/src/buildstream/testing/_sourcetests/workspace.py @@ -67,33 +67,14 @@ class WorkspaceCreator: if element_attrs: element = {**element, **element_attrs} _yaml.roundtrip_dump(element, os.path.join(element_path, element_name)) - return element_name, element_path, workspace_dir - - def create_workspace_elements(self, kinds, suffixs=None, workspace_dir_usr=None, element_attrs=None): - - element_tuples = [] - - if suffixs is None: - suffixs = ["",] * len(kinds) - else: - if len(suffixs) != len(kinds): - raise "terable error" - - for suffix, kind in zip(suffixs, kinds): - element_name, _, workspace_dir = self.create_workspace_element( - kind, suffix, workspace_dir_usr, element_attrs - ) - element_tuples.append((element_name, workspace_dir)) # Assert that there is no reference, a fetch is needed - states = self.cli.get_element_states(self.project_path, [e for e, _ in element_tuples]) - assert not any(states[e] != "fetch needed" for e, _ in element_tuples) - - return element_tuples + assert self.cli.get_element_state(self.project_path, element_name) == "fetch needed" + return element_name, workspace_dir - def open_workspaces(self, kinds, suffixs=None, workspace_dir=None, element_attrs=None, no_checkout=False): + def open_workspace(self, kind, suffix=None, workspace_dir=None, element_attrs=None, no_checkout=False): - element_tuples = self.create_workspace_elements(kinds, suffixs, workspace_dir, element_attrs) + element_name, workspace_dir = self.create_workspace_element(kind, suffix, workspace_dir, element_attrs) os.makedirs(self.workspace_cmd, exist_ok=True) # Now open the workspace, this should have the effect of automatically @@ -103,46 +84,23 @@ class WorkspaceCreator: if no_checkout: args.append("--no-checkout") if workspace_dir is not None: - assert len(element_tuples) == 1, "test logic error" - _, workspace_dir = element_tuples[0] args.extend(["--directory", workspace_dir]) - args.extend([element_name for element_name, workspace_dir_suffix in element_tuples]) + args.append(element_name) result = self.cli.run(cwd=self.workspace_cmd, project=self.project_path, args=args) result.assert_success() if not no_checkout: # Assert that we are now buildable because the source is now cached. - states = self.cli.get_element_states(self.project_path, [e for e, _ in element_tuples]) - assert not any(states[e] != "buildable" for e, _ in element_tuples) + assert self.cli.get_element_state(self.project_path, element_name) == "buildable" # Check that the executable hello file is found in each workspace - for _, workspace in element_tuples: - filename = os.path.join(workspace, "usr", "bin", "hello") - assert os.path.exists(filename) - - return element_tuples - - -def open_workspace( - cli, - tmpdir, - datafiles, - kind, - suffix="", - workspace_dir=None, - project_path=None, - element_attrs=None, - no_checkout=False, -): - workspace_object = WorkspaceCreator(cli, tmpdir, datafiles, project_path) - workspaces = workspace_object.open_workspaces((kind,), (suffix,), workspace_dir, element_attrs, no_checkout) - assert len(workspaces) == 1 - element_name, workspace = workspaces[0] - return element_name, workspace_object.project_path, workspace + filename = os.path.join(workspace_dir, "usr", "bin", "hello") + assert os.path.exists(filename) @pytest.mark.datafiles(DATA_DIR) def test_open(cli, tmpdir, datafiles, kind): - open_workspace(cli, tmpdir, datafiles, kind) + workspace_object = WorkspaceCreator(cli, tmpdir, datafiles) + workspace_object.open_workspace(kind) -- cgit v1.2.1 From 4eaf1ba5ae52d722b9413af0fc0f9285ea0ae886 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Thu, 8 Oct 2020 21:30:06 +0000 Subject: workspace.py: Remove arguments that are unused during the test --- src/buildstream/testing/_sourcetests/workspace.py | 46 ++++++++--------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/src/buildstream/testing/_sourcetests/workspace.py b/src/buildstream/testing/_sourcetests/workspace.py index 7f3c5dea4..ebc03c97d 100644 --- a/src/buildstream/testing/_sourcetests/workspace.py +++ b/src/buildstream/testing/_sourcetests/workspace.py @@ -20,7 +20,6 @@ # pylint: disable=redefined-outer-name import os -import shutil import pytest from buildstream import _yaml @@ -34,28 +33,20 @@ DATA_DIR = os.path.join(TOP_DIR, "project") class WorkspaceCreator: - def __init__(self, cli, tmpdir, datafiles, project_path=None): + def __init__(self, cli, tmpdir, datafiles): self.cli = cli self.tmpdir = tmpdir - self.datafiles = datafiles - if not project_path: - project_path = str(datafiles) - else: - shutil.copytree(str(datafiles), project_path) - - self.project_path = project_path - self.bin_files_path = os.path.join(project_path, "files", "bin-files") + self.project_path = str(datafiles) + self.bin_files_path = os.path.join(self.project_path, "files", "bin-files") self.workspace_cmd = os.path.join(self.project_path, "workspace_cmd") - def create_workspace_element(self, kind, suffix="", workspace_dir=None, element_attrs=None): - element_name = "workspace-test-{}{}.bst".format(kind, suffix) + def create_workspace_element(self, kind): + element_name = "workspace-test-{}.bst".format(kind) element_path = os.path.join(self.project_path, "elements") - if not workspace_dir: - workspace_dir = os.path.join(self.workspace_cmd, element_name) - if workspace_dir[-4:] == ".bst": - workspace_dir = workspace_dir[:-4] + # remove the '.bst' at the end of the element + workspace_dir = os.path.join(self.workspace_cmd, element_name[:-4]) # Create our repo object of the given source type with # the bin files, and then collect the initial ref. @@ -64,40 +55,33 @@ class WorkspaceCreator: # Write out our test target element = {"kind": "import", "sources": [repo.source_config(ref=ref)]} - if element_attrs: - element = {**element, **element_attrs} _yaml.roundtrip_dump(element, os.path.join(element_path, element_name)) # Assert that there is no reference, a fetch is needed assert self.cli.get_element_state(self.project_path, element_name) == "fetch needed" return element_name, workspace_dir - def open_workspace(self, kind, suffix=None, workspace_dir=None, element_attrs=None, no_checkout=False): + def open_workspace(self, kind): - element_name, workspace_dir = self.create_workspace_element(kind, suffix, workspace_dir, element_attrs) + element_name, workspace_dir = self.create_workspace_element(kind) os.makedirs(self.workspace_cmd, exist_ok=True) # Now open the workspace, this should have the effect of automatically # fetching the source from the repo. args = ["workspace", "open"] - - if no_checkout: - args.append("--no-checkout") - if workspace_dir is not None: - args.extend(["--directory", workspace_dir]) + args.extend(["--directory", workspace_dir]) args.append(element_name) result = self.cli.run(cwd=self.workspace_cmd, project=self.project_path, args=args) result.assert_success() - if not no_checkout: - # Assert that we are now buildable because the source is now cached. - assert self.cli.get_element_state(self.project_path, element_name) == "buildable" + # Assert that we are now buildable because the source is now cached. + assert self.cli.get_element_state(self.project_path, element_name) == "buildable" - # Check that the executable hello file is found in each workspace - filename = os.path.join(workspace_dir, "usr", "bin", "hello") - assert os.path.exists(filename) + # Check that the executable hello file is found in each workspace + filename = os.path.join(workspace_dir, "usr", "bin", "hello") + assert os.path.exists(filename) @pytest.mark.datafiles(DATA_DIR) -- cgit v1.2.1 From f1986c135c8bc8132c15c4b45230e872d13ad60a Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Fri, 9 Oct 2020 07:59:23 +0000 Subject: workspace.py: Remove some abstractions and write the whole test in place This is to remove some hoops that are currently used, that do not fit well with most of the style in the code. It also removes indirections and make the test easier to understand --- src/buildstream/testing/_sourcetests/workspace.py | 77 +++++++++-------------- 1 file changed, 31 insertions(+), 46 deletions(-) diff --git a/src/buildstream/testing/_sourcetests/workspace.py b/src/buildstream/testing/_sourcetests/workspace.py index ebc03c97d..a171bf703 100644 --- a/src/buildstream/testing/_sourcetests/workspace.py +++ b/src/buildstream/testing/_sourcetests/workspace.py @@ -32,59 +32,44 @@ TOP_DIR = os.path.dirname(os.path.realpath(__file__)) DATA_DIR = os.path.join(TOP_DIR, "project") -class WorkspaceCreator: - def __init__(self, cli, tmpdir, datafiles): - self.cli = cli - self.tmpdir = tmpdir - - self.project_path = str(datafiles) - self.bin_files_path = os.path.join(self.project_path, "files", "bin-files") - - self.workspace_cmd = os.path.join(self.project_path, "workspace_cmd") - - def create_workspace_element(self, kind): - element_name = "workspace-test-{}.bst".format(kind) - element_path = os.path.join(self.project_path, "elements") - # remove the '.bst' at the end of the element - workspace_dir = os.path.join(self.workspace_cmd, element_name[:-4]) - - # Create our repo object of the given source type with - # the bin files, and then collect the initial ref. - repo = create_repo(kind, str(self.tmpdir)) - ref = repo.create(self.bin_files_path) - - # Write out our test target - element = {"kind": "import", "sources": [repo.source_config(ref=ref)]} - _yaml.roundtrip_dump(element, os.path.join(element_path, element_name)) +@pytest.mark.datafiles(DATA_DIR) +def test_open(cli, tmpdir, datafiles, kind): + project_path = str(datafiles) + bin_files_path = os.path.join(project_path, "files", "bin-files") - # Assert that there is no reference, a fetch is needed - assert self.cli.get_element_state(self.project_path, element_name) == "fetch needed" - return element_name, workspace_dir + element_name = "workspace-test-{}.bst".format(kind) + element_path = os.path.join(project_path, "elements") - def open_workspace(self, kind): + # Create our repo object of the given source type with + # the bin files, and then collect the initial ref. + repo = create_repo(kind, str(tmpdir)) + ref = repo.create(bin_files_path) - element_name, workspace_dir = self.create_workspace_element(kind) - os.makedirs(self.workspace_cmd, exist_ok=True) + # Write out our test target + element = {"kind": "import", "sources": [repo.source_config(ref=ref)]} + _yaml.roundtrip_dump(element, os.path.join(element_path, element_name)) - # Now open the workspace, this should have the effect of automatically - # fetching the source from the repo. - args = ["workspace", "open"] - args.extend(["--directory", workspace_dir]) + # Assert that there is no reference, a fetch is needed + assert cli.get_element_state(project_path, element_name) == "fetch needed" - args.append(element_name) - result = self.cli.run(cwd=self.workspace_cmd, project=self.project_path, args=args) + workspace_cmd = os.path.join(project_path, "workspace_cmd") + os.makedirs(workspace_cmd, exist_ok=True) + # remove the '.bst' at the end of the element + workspace_dir = os.path.join(workspace_cmd, element_name[:-4]) - result.assert_success() + # Now open the workspace, this should have the effect of automatically + # fetching the source from the repo. + args = ["workspace", "open"] + args.extend(["--directory", workspace_dir]) - # Assert that we are now buildable because the source is now cached. - assert self.cli.get_element_state(self.project_path, element_name) == "buildable" + args.append(element_name) + result = cli.run(cwd=workspace_cmd, project=project_path, args=args) - # Check that the executable hello file is found in each workspace - filename = os.path.join(workspace_dir, "usr", "bin", "hello") - assert os.path.exists(filename) + result.assert_success() + # Assert that we are now buildable because the source is now cached. + assert cli.get_element_state(project_path, element_name) == "buildable" -@pytest.mark.datafiles(DATA_DIR) -def test_open(cli, tmpdir, datafiles, kind): - workspace_object = WorkspaceCreator(cli, tmpdir, datafiles) - workspace_object.open_workspace(kind) + # Check that the executable hello file is found in each workspace + filename = os.path.join(workspace_dir, "usr", "bin", "hello") + assert os.path.exists(filename) -- cgit v1.2.1 From 1263d86e06390d49a92666ed31a36a5d6338eea0 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Fri, 9 Oct 2020 08:10:28 +0000 Subject: workspace.py: Simplify invocation of the cli command --- src/buildstream/testing/_sourcetests/workspace.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/buildstream/testing/_sourcetests/workspace.py b/src/buildstream/testing/_sourcetests/workspace.py index a171bf703..64b59d865 100644 --- a/src/buildstream/testing/_sourcetests/workspace.py +++ b/src/buildstream/testing/_sourcetests/workspace.py @@ -59,11 +59,7 @@ def test_open(cli, tmpdir, datafiles, kind): # Now open the workspace, this should have the effect of automatically # fetching the source from the repo. - args = ["workspace", "open"] - args.extend(["--directory", workspace_dir]) - - args.append(element_name) - result = cli.run(cwd=workspace_cmd, project=project_path, args=args) + result = cli.run(project=project_path, args=["workspace", "open", "--directory", workspace_dir, element_name]) result.assert_success() -- cgit v1.2.1 From 82e026543d218df258faaa7a2b2ff3f725b5f415 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Fri, 9 Oct 2020 10:53:24 +0000 Subject: workspace.py: Don't use the same directory for everything --- src/buildstream/testing/_sourcetests/workspace.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/buildstream/testing/_sourcetests/workspace.py b/src/buildstream/testing/_sourcetests/workspace.py index 64b59d865..9edd9d994 100644 --- a/src/buildstream/testing/_sourcetests/workspace.py +++ b/src/buildstream/testing/_sourcetests/workspace.py @@ -33,7 +33,7 @@ DATA_DIR = os.path.join(TOP_DIR, "project") @pytest.mark.datafiles(DATA_DIR) -def test_open(cli, tmpdir, datafiles, kind): +def test_open(cli, tmpdir_factory, datafiles, kind): project_path = str(datafiles) bin_files_path = os.path.join(project_path, "files", "bin-files") @@ -42,7 +42,7 @@ def test_open(cli, tmpdir, datafiles, kind): # Create our repo object of the given source type with # the bin files, and then collect the initial ref. - repo = create_repo(kind, str(tmpdir)) + repo = create_repo(kind, str(tmpdir_factory.mktemp("repo-{}".format(kind)))) ref = repo.create(bin_files_path) # Write out our test target @@ -52,10 +52,7 @@ def test_open(cli, tmpdir, datafiles, kind): # Assert that there is no reference, a fetch is needed assert cli.get_element_state(project_path, element_name) == "fetch needed" - workspace_cmd = os.path.join(project_path, "workspace_cmd") - os.makedirs(workspace_cmd, exist_ok=True) - # remove the '.bst' at the end of the element - workspace_dir = os.path.join(workspace_cmd, element_name[:-4]) + workspace_dir = os.path.join(tmpdir_factory.mktemp("opened_workspace")) # Now open the workspace, this should have the effect of automatically # fetching the source from the repo. -- cgit v1.2.1