Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workspace.py: Remove assumption on specific kinds to testing multiple #1566

Merged
merged 1 commit into from
Jan 19, 2022

Conversation

BenjaminSchubert
Copy link
Contributor

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

@gtristan
Copy link
Contributor

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.

@BenjaminSchubert
Copy link
Contributor Author

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 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
@gtristan
Copy link
Contributor

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.

@BenjaminSchubert
Copy link
Contributor Author

@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!

@BenjaminSchubert BenjaminSchubert merged commit ea34d5f into master Jan 19, 2022
@BenjaminSchubert BenjaminSchubert deleted the bschubert/rewrite-workspace-multi-test branch January 19, 2022 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants