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

Simplify source workspace tests #1432

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Cynical-Optimist
Copy link

See original merge request on GitLab
In GitLab by [Gitlab user @BenjaminSchubert] on Oct 9, 2020, 12:46

Description

This simplifies the source workspace tests, by removing a lot of the complexity that got brought in by copying the workspace element tests

This test seems to be copy pasted from our tests around workspaces and
doesn't need to be so complicated
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
@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @juergbi] on Oct 13, 2020, 17:22

Commented on src/buildstream/testing/_sourcetests/workspace.py line 49

-4 is on the wrong side of the colon. That said, it's removed anyway in a follow-up commit.

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @juergbi] on Oct 13, 2020, 17:23

approved this merge request

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @BenjaminSchubert] on Oct 13, 2020, 17:26

Commented on src/buildstream/testing/_sourcetests/workspace.py line 49

Ah good catch, i'll fix this and send for a merge

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @BenjaminSchubert] on Oct 13, 2020, 17:43

added 4 commits

  • 0e69e9b5 - workspace.py: Remove arguments that are unused during the test
  • 7e1918c8 - workspace.py: Remove some abstractions and write the whole test in place
  • d21ab74e - workspace.py: Simplify invocation of the cli command
  • d7313adf - workspace.py: Don't use the same directory for everything

Compare with previous version

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @BenjaminSchubert] on Oct 13, 2020, 17:47

resolved all threads

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @BenjaminSchubert] on Oct 13, 2020, 17:47

assigned to [Gitlab user @marge-bot123]

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @marge-bot123] on Oct 13, 2020, 19:48

assigned to [Gitlab user @BenjaminSchubert] and unassigned [Gitlab user @marge-bot123]

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @marge-bot123] on Oct 13, 2020, 19:48

I couldn't merge this branch: CI is taking too long.

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @BenjaminSchubert] on Oct 13, 2020, 21:35

assigned to [Gitlab user @marge-bot123]

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @marge-bot123] on Oct 13, 2020, 23:35

unassigned [Gitlab user @marge-bot123]

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @marge-bot123] on Oct 13, 2020, 23:35

I couldn't merge this branch: CI is taking too long.

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.

2 participants