8:03 p.m.
Repo: git.baserock.org:baserock/baserock/morph
Branch: pedroalvarez/S9428
Sha1: b5a6b47cdd5eec260d854e728a88a341a69922ef
Card: S9428
Pedro Alvarez (2):
Don't remove an existing branch in 'morph branch'
Add regression test, morph branch doesn't remove existing branch dir
morphlib/plugins/branch_and_merge_new_plugin.py | 3 +++
yarns/regression.yarn | 12 ++++++++++++
2 files changed, 15 insertions(+)
--
1.7.10.4
8:03 p.m.
New subject: [PATCH 1/2] Don't remove an existing branch in 'morph branch'
---
morphlib/plugins/branch_and_merge_new_plugin.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/morphlib/plugins/branch_and_merge_new_plugin.py
b/morphlib/plugins/branch_and_merge_new_plugin.py
index 9a0ee48..dba529d 100644
--- a/morphlib/plugins/branch_and_merge_new_plugin.py
+++ b/morphlib/plugins/branch_and_merge_new_plugin.py
@@ -134,6 +134,9 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin):
gd.update_submodules(self.app)
gd.update_remotes()
+ except morphlib.sysbranchdir.SystemBranchDirectoryAlreadyExists as e:
+ logging.error('Caught exception: %s' % str(e))
+ raise
except BaseException as e:
# Oops. Clean up.
logging.error('Caught exception: %s' % str(e))
--
1.7.10.4
8:03 p.m.
New subject: [PATCH 2/2] Add regression test, morph branch doesn't remove existing branch dir
---
yarns/regression.yarn | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/yarns/regression.yarn b/yarns/regression.yarn
index d1cc8e7..e93b8b6 100644
--- a/yarns/regression.yarn
+++ b/yarns/regression.yarn
@@ -12,3 +12,15 @@ Testing if we can build after checking out from a tag.
WHEN checking out the test-tag system tag
THEN morph build the system simple-system of the tag test-tag of the repo
test:morphs
+
+Running `morph branch` when the branch directory exists don't
+remove the existing directory.
+
+ SCENARIO morph branch don't remove an existing branch directory
+ GIVEN a workspace
+ AND a git server
+ WHEN creating system branch foo
+ THEN the system branch foo is checked out
+ WHEN morph attempts to check out system branch foo
+ THEN the system branch foo is checked out
+ AND morph failed
--
1.7.10.4
8:09 p.m.
New subject: [PATCH 2/2] Add regression test, morph branch doesn't remove existing branch dir
On Wed, Oct 30, 2013 at 11:03:35AM +0000, Pedro Alvarez wrote:
<snip>
+Running `morph branch` when the branch directory exists don't
+remove the existing directory.
+
+ SCENARIO morph branch don't remove an existing branch directory
+ GIVEN a workspace
+ AND a git server
+ WHEN creating system branch foo
+ THEN the system branch foo is checked out
+ WHEN morph attempts to check out system branch foo
+ THEN the system branch foo is checked out
+ AND morph failed
s/don't/doesn't/g
Interesting bug. We definitely need this test, so you've got a +1 from
me if you do the language tweak, but I'm wondering if it may be worth
adding it to the scenario that does the checkout initially, rather than
adding a new scenario.
8:10 p.m.
New subject: [PATCH 2/2] Add regression test, morph branch doesn't remove existing branch dir
On Wed, Oct 30, 2013 at 11:03:35 +0000, Pedro Alvarez wrote:
+Running `morph branch` when the branch directory exists don't
+remove the existing directory.
I feel like...
When running `morph branch` to create a branch which already exists in the
workspace, morph will encounter an error. Normally on error, morph cleans up
the branch which it was trying to create. We do **NOT** want it to clean up a
branch which already existed. This test ensures that does not occur.
...might be a better explanation for the test. We should be clear in
regression test descriptions what is going on and why we're testing.
--
Daniel Silverstone http://www.codethink.co.uk/
Programme Manager - Baserock GPG 4096/R Key Id: 3CCE BABE 206C 3B69
8:15 p.m.
New subject: [PATCH 2/2] Add regression test, morph branch doesn't remove existing branch dir
On 30/10/13 11:03, Pedro Alvarez wrote:
+
+ SCENARIO morph branch don't remove an existing branch directory
+ GIVEN a workspace
+ AND a git server
+ WHEN creating system branch foo
+ THEN the system branch foo is checked out
I think this THEN is unnecessary, either we already test for this
behaviour or it's implicit.
+ WHEN morph attempts to check out system branch foo
+ THEN the system branch foo is checked out
+ AND morph failed
If I understood the bug correctly, then
WHEN creating system branch foo
AND morph attempt to checkout the system branch foo
THEN morph failed
AND the system branch foo is checked out
is clearer.
Please correct me if I misunderstood.
8:26 p.m.
New subject: [PATCH 2/2] Add regression test, morph branch doesn't remove existing branch dir
On 30/10/13 11:15, Dan Firth wrote:
If I understood the bug correctly, then
WHEN creating system branch foo
AND morph attempt to checkout the system branch foo
THEN morph failed
AND the system branch foo is checked out
is clearer.
Please correct me if I misunderstood.
Really good idea, I think is going to be clearer to understand
Thanks
8:25 p.m.
New subject: [PATCH 2/2] Add regression test, morph branch doesn't remove existing branch dir
On 30/10/2013 11:03, Pedro Alvarez wrote:
<snip>
+
+Running `morph branch` when the branch directory exists don't
+remove the existing directory.
+
+ SCENARIO morph branch don't remove an existing branch directory
+ GIVEN a workspace
+ AND a git server
+ WHEN creating system branch foo
+ THEN the system branch foo is checked out
+ WHEN morph attempts to check out system branch foo
+ THEN the system branch foo is checked out
+ AND morph failed
Here's my attempt:
SCENARIO re-running 'morph branch' fails, original branch untouched
GIVEN a workspace
AND a git server
WHEN user creates system branch foo
THEN system branch foo is checked out
WHEN user modifies system branch foo
AND user creates system branch foo
THEN morph fails because foo exists
AND modified system branch foo is still checked out
-
12:33 a.m.
New subject: [PATCHv2 0/3] Now `morph branch` doesn't remove an existing branch directory
Repo: git.baserock.org:baserock/baserock/morph
Branch: pedroalvarez/S9428
Sha1: 8504f029bd2dffd1550b16c50d293aa58290d333
Card: S9428
Is it better now? Suggestions?
I have also refactorized some test, to make this one more readable.
Pedro Alvarez (3):
Don't remove an existing branch in 'morph branch'
Some tests refactorized for better understanding.
Add regression test, morph branch doesn't remove existing branch dir
morphlib/plugins/branch_and_merge_new_plugin.py | 3 +++
yarns/branches-workspaces.yarn | 22 +++++++++++-----------
yarns/implementations.yarn | 6 +++++-
yarns/regression.yarn | 20 ++++++++++++++++++++
4 files changed, 39 insertions(+), 12 deletions(-)
--
1.7.10.4
12:33 a.m.
New subject: [PATCH 1/3] Don't remove an existing branch in 'morph branch'
---
morphlib/plugins/branch_and_merge_new_plugin.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/morphlib/plugins/branch_and_merge_new_plugin.py
b/morphlib/plugins/branch_and_merge_new_plugin.py
index 9a0ee48..dba529d 100644
--- a/morphlib/plugins/branch_and_merge_new_plugin.py
+++ b/morphlib/plugins/branch_and_merge_new_plugin.py
@@ -134,6 +134,9 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin):
gd.update_submodules(self.app)
gd.update_remotes()
+ except morphlib.sysbranchdir.SystemBranchDirectoryAlreadyExists as e:
+ logging.error('Caught exception: %s' % str(e))
+ raise
except BaseException as e:
# Oops. Clean up.
logging.error('Caught exception: %s' % str(e))
--
1.7.10.4
12:33 a.m.
New subject: [PATCH 2/3] Some tests refactorized for better understanding.
---
yarns/branches-workspaces.yarn | 22 +++++++++++-----------
yarns/implementations.yarn | 6 +++++-
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/yarns/branches-workspaces.yarn b/yarns/branches-workspaces.yarn
index cd3f7a0..b90c054 100644
--- a/yarns/branches-workspaces.yarn
+++ b/yarns/branches-workspaces.yarn
@@ -72,7 +72,7 @@ We can, instead, create a new system branch, off master.
SCENARIO branch off master
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
THEN the system branch foo is checked out
We can also branch off another system branch. However, we need to first
@@ -82,7 +82,7 @@ to check for that locally.
SCENARIO branch off non-master
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
AND pushing system branch foo to git server
AND creating system branch bar, based on foo
THEN the system branch bar is checked out
@@ -147,7 +147,7 @@ current directory, things should fail.
GIVEN a workspace
AND a git server
WHEN checking out the master system branch
- AND creating system branch foo
+ AND user creates system branch foo
AND attempting to report system branch in .
THEN morph failed
@@ -186,7 +186,7 @@ all the refs are unchanged.
SCENARIO morph branch does not edit refs
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
THEN in branch foo, system test-system refs test-stratum in master
AND in branch foo, stratum test-stratum refs test-chunk in master
@@ -214,7 +214,7 @@ repositories referenced in the system branch.
SCENARIO morph status reports changes correctly
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
THEN morph reports no outstanding changes in foo
WHEN editing stratum test-stratum in system test-system in branch foo
@@ -244,7 +244,7 @@ branch checkout.
SCENARIO morph foreach runs command in each git repo
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
AND editing chunk test-chunk in test-stratum in test-system in branch foo
AND running shell command in each repo in foo
THEN morph ran command in test:morphs in foo
@@ -261,7 +261,7 @@ unpetrify and verify that we have all the same refs as before.
SCENARIO morph petrifies and unpetrifies
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
AND pushing system branch foo to git server
AND remembering all refs in foo
AND petrifying foo
@@ -279,7 +279,7 @@ system branch itself, only the tag.
SCENARIO morph tags a system branch
GIVEN a workspace
AND a git server
- WHEN creating system branch foo
+ WHEN user creates system branch foo
AND tagging system branch foo as test123
THEN morph tag test123 in foo is an annotated git tag
AND morph tag test123 in foo refers to a petrified commit
@@ -314,7 +314,7 @@ wouldn't need to worry about changing the system branch ref.
GIVEN a workspace
AND a git server
AND null refs for local strata
- WHEN creating system branch foo
+ WHEN user creates system branch foo
THEN the system branch foo is checked out
When we edit a morphology with null refs, they stay null.
@@ -326,7 +326,7 @@ When we edit a morphology with null refs, they stay null.
When creating the branch, the refs remain null.
- WHEN creating system branch foo
+ WHEN user creates system branch foo
THEN in branch foo, system test-system refs test-stratum in None
After editing the stratum they remain null.
@@ -347,7 +347,7 @@ Petrifying also leaves null refs unmolested
GIVEN a workspace
AND a git server
AND null refs for local strata
- WHEN creating system branch foo
+ WHEN user creates system branch foo
AND pushing system branch foo to git server
AND remembering all refs in foo
AND petrifying foo
diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn
index 2212013..985ab2b 100644
--- a/yarns/implementations.yarn
+++ b/yarns/implementations.yarn
@@ -183,10 +183,14 @@ We also need to verify that a system branch has been checked out.
We can create a new branch, off master.
- IMPLEMENTS WHEN creating system branch (\S+)
+ IMPLEMENTS WHEN user creates system branch (\S+)
cd "$DATADIR/workspace"
run_morph branch test:morphs "$MATCH_1"
+ IMPLEMENTS WHEN user attempts to create system branch (\S+)
+ cd "$DATADIR/workspace"
+ attempt_morph branch test:morphs "$MATCH_1"
+
We can create a new branch, off another system branch.
IMPLEMENTS WHEN creating system branch (\S+), based on (\S+)
--
1.7.10.4
12:33 a.m.
New subject: [PATCH 3/3] Add regression test, morph branch doesn't remove existing branch dir
---
yarns/regression.yarn | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/yarns/regression.yarn b/yarns/regression.yarn
index d1cc8e7..7480655 100644
--- a/yarns/regression.yarn
+++ b/yarns/regression.yarn
@@ -12,3 +12,23 @@ Testing if we can build after checking out from a tag.
WHEN checking out the test-tag system tag
THEN morph build the system simple-system of the tag test-tag of the repo
test:morphs
+
+Running `morph branch` when the branch directory exists doesn't
+remove the existing directory.
+
+ SCENARIO re-running 'morph branch' fails, original branch untouched
+ GIVEN a workspace
+ AND a git server
+ WHEN user creates system branch foo
+ THEN the system branch foo is checked out
+
+The branch is checked out correctly, now it should fail if the user executes
+`morph branch` with the same branch name.
+
+ WHEN user attempts to create system branch foo
+ THEN morph failed
+ AND the branch error message includes the string "File exists"
+
+The branch still checked out.
+
+ AND the system branch foo is checked out
--
1.7.10.4
11:43 p.m.
New subject: [PATCHv2 0/3] Now `morph branch` doesn't remove an existing branch directory
On Thu, Oct 31, 2013 at 03:33:11PM +0000, Pedro Alvarez wrote:
Repo: git.baserock.org:baserock/baserock/morph
Branch: pedroalvarez/S9428
Sha1: 8504f029bd2dffd1550b16c50d293aa58290d333
Card: S9428
Merged! Thank you, Pedro.
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
3618
days inactive
3620
days old
12 comments
6 participants
participants (6)
-
Dan Firth -
Daniel Silverstone -
Lars Wirzenius -
Paul Sherwood -
Pedro Alvarez -
Richard Maw