You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by GitBox <gi...@apache.org> on 2022/01/15 16:17:57 UTC
[GitHub] [buildstream] BenjaminSchubert opened a new pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
BenjaminSchubert opened a new pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566
This makes the test more explicit in testing that it can actually open
multiple different projects at the same time, and removes the need of
actually using multiple kinds
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] juergbi commented on a change in pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
juergbi commented on a change in pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#discussion_r785331196
##########
File path: tests/frontend/workspace.py
##########
@@ -175,20 +187,13 @@ def test_open_bzr_customize(cli, tmpdir, datafiles):
@pytest.mark.datafiles(DATA_DIR)
def test_open_multi(cli, tmpdir, datafiles):
-
workspace_object = WorkspaceCreator(cli, tmpdir, datafiles)
workspaces = workspace_object.open_workspaces(repo_kinds)
for (elname, workspace), kind in zip(workspaces, repo_kinds):
assert kind in elname
workspace_lsdir = os.listdir(workspace)
- if kind == "git":
- assert ".git" in workspace_lsdir
Review comment:
Do we lose the check that a workspace on a git source has a `.git` directory? Or is such a check irrelevant or redundant (it might make sense to check this in a plugin-specific test)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
gtristan commented on pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#issuecomment-1016051860
> > We added tests to ensure that by default we don’t stage the git/bzr history (when we made that behavior default), we should keep these assertions in some form somewhere.
>
> This PR doesn't change this. It merely removes a fuzzy validation that the workspace was opened correctly. We have tests in each of those sources that make sure of this.
Technically this testing should be in the domain of the sources themselves, but do remember that some sources (probably `git` and `bzr` included) use *different* codepaths for opening a workspace than they do for merely staging sources (see `Source.init_workspace()`).
If this assertion exists for opening workspaces on `git` / `bzr` *somewhere* then lets immediately just remove these.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
gtristan commented on pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#issuecomment-1013761916
We added tests to ensure that by default we don’t stage the git/bzr history (when we made that behavior default), we should keep these assertions in some form somewhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] juergbi commented on a change in pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
juergbi commented on a change in pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#discussion_r785332661
##########
File path: tests/frontend/workspace.py
##########
@@ -175,20 +187,13 @@ def test_open_bzr_customize(cli, tmpdir, datafiles):
@pytest.mark.datafiles(DATA_DIR)
def test_open_multi(cli, tmpdir, datafiles):
-
workspace_object = WorkspaceCreator(cli, tmpdir, datafiles)
workspaces = workspace_object.open_workspaces(repo_kinds)
for (elname, workspace), kind in zip(workspaces, repo_kinds):
assert kind in elname
workspace_lsdir = os.listdir(workspace)
- if kind == "git":
- assert ".git" in workspace_lsdir
Review comment:
Tests that run `git`/`bzr` on the workspace directory should indeed be sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] BenjaminSchubert commented on pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
BenjaminSchubert commented on pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#issuecomment-1016872214
@gtristan absolutely:
For git, we have https://github.com/apache/buildstream/blob/master/tests/sources/git.py#L64-L86 that ensures we can successfully run `git`.
For `bzr`, we have https://github.com/apache/buildstream/pull/1565/files that also tangentially ensures we have a .bzr directory.
So merging this for now :) thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] BenjaminSchubert merged pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
BenjaminSchubert merged pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] BenjaminSchubert commented on a change in pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
BenjaminSchubert commented on a change in pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#discussion_r785332152
##########
File path: tests/frontend/workspace.py
##########
@@ -175,20 +187,13 @@ def test_open_bzr_customize(cli, tmpdir, datafiles):
@pytest.mark.datafiles(DATA_DIR)
def test_open_multi(cli, tmpdir, datafiles):
-
workspace_object = WorkspaceCreator(cli, tmpdir, datafiles)
workspaces = workspace_object.open_workspaces(repo_kinds)
for (elname, workspace), kind in zip(workspaces, repo_kinds):
assert kind in elname
workspace_lsdir = os.listdir(workspace)
- if kind == "git":
- assert ".git" in workspace_lsdir
Review comment:
For git, we have https://github.com/apache/buildstream/blob/master/tests/sources/git.py#L64-L86 that ensures we can successfully run `git`.
For `bzr`, we have https://github.com/apache/buildstream/pull/1565/files that also tangentially ensures we have a .bzr directory.
If you think it's better to have explicit tests, I am happy to add some in the source-specific entries.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] BenjaminSchubert commented on pull request #1566: workspace.py: Remove assumption on specific kinds to testing multiple
Posted by GitBox <gi...@apache.org>.
BenjaminSchubert commented on pull request #1566:
URL: https://github.com/apache/buildstream/pull/1566#issuecomment-1013902810
> We added tests to ensure that by default we don’t stage the git/bzr history (when we made that behavior default), we should keep these assertions in some form somewhere.
This PR doesn't change this. It merely removes a fuzzy validation that the workspace was opened correctly. We have tests in each of those sources that make sure of this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org