Daniel Shahaf <d...@daniel.shahaf.name> writes:
> Noorul Islam K M wrote on Fri, Feb 25, 2011 at 12:09:57 +0530:
>
>> Log
>> [[[
>>
>> 'svn up' fails if external propset is not committed.
>>
>
> This is an English sentence, but it is not appropriate as the first
> ("overview") paragraph of the log message since it doesn't describe the
> change being made by the patch.
>
>> * subversion/tests/cmdline/externals_tests.py
>> (file_external_in_sibling): Remove TODO entry.
>> (file_external_update_without_commit): New test.
>> (test_list): Run it.
>>
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> ]]]
>>
>
>> Index: subversion/tests/cmdline/externals_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/externals_tests.py (revision 1074405)
>> +++ subversion/tests/cmdline/externals_tests.py (working copy)
>> @@ -1678,13 +1678,23 @@
>> change_external(sbox.ospath('A2'), externals_prop)
>> sbox.simple_update()
>>
>> - # TODO: Currently, 'svn up' would fail if change_external() didn't commit
>> - # its change. That needs a separate test...
>> -
>> expected_stdout = ["Updating '.' ...\n", "At revision 2.\n"]
>> os.chdir(sbox.ospath("A"))
>> svntest.actions.run_and_verify_svn(None, expected_stdout, [], 'update')
>>
>> +@XFail()
>> +def file_external_update_without_commit(sbox):
>
> Per recent threads, all XFail tests should have an associated issue
> (i.e., an @Issue() decorator). Could you file an issue or point to an
> existing issue we can add here?
>
>> + "update a file external without committing"
>> +
>
> The summary does not point out that it's the addition of A2, rather than
> the setting of a property on it, which hadn't been committed. IMO it
> would be good to point that out.
>
>> + sbox.build()
>
> I added 'read_only=True' (and then committed r1074488 to avoid it
> spuriously triggerring an assertion).
>
>> + wc_dir = sbox.wc_dir
>> +
>
> Unused variable.
>
>> + # Setup A2/iota as file external to ^/iota
>> + externals_prop = "^/iota iota\n"
>> + sbox.simple_mkdir("A2")
>> + change_external(sbox.ospath('A2'), externals_prop, commit=False)
>> + sbox.simple_update()
>> +
>> ########################################################################
>> # Run the tests
>>
>> @@ -1719,6 +1729,7 @@
>> update_external_on_locally_added_dir,
>> switch_external_on_locally_added_dir,
>> file_external_in_sibling,
>> + file_external_update_without_commit,
>> ]
>>
>> if __name__ == '__main__':
>
>
> Committed in r1074492, thanks. Please follow up on the two points
> I mentioned earlier (issue number and test name/description).
Thank you for reviewing.
Attached is the follow-up patch incorporating your reviews comments.
Log
[[[
Follow-up to r1074492.
* subversion/tests/cmdline/externals_tests.py
(file_external_update_without_commit): Update test summary. Remove
unused variable. Associate test with issue 3823.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
Suggested by: danielsh
]]]
Index: subversion/tests/cmdline/externals_tests.py
===================================================================
--- subversion/tests/cmdline/externals_tests.py (revision 1074498)
+++ subversion/tests/cmdline/externals_tests.py (working copy)
@@ -1683,11 +1683,11 @@
svntest.actions.run_and_verify_svn(None, expected_stdout, [], 'update')
@XFail()
+@Issue(3823)
def file_external_update_without_commit(sbox):
- "update a file external without committing"
+ "update a file external without committing target"
sbox.build(read_only=True)
- wc_dir = sbox.wc_dir
# Setup A2/iota as file external to ^/iota
externals_prop = "^/iota iota\n"
