On Tue, Apr 8, 2008 at 11:16 PM, Aaron Bentley <<a href="mailto:aaron@aaronbentley.com">aaron@aaronbentley.com</a>> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
bb:resubmit<br>
<br>
I'd like to see this platform detection factored out into an osutils<br>
method. We shouldn't be repeating ourselves here. Maybe<br>
osutils.apps_support_symlinks<br>
</blockquote><div><br>Will do. How about osutils.host_os_can_dereference_symlinks? ...though the new method may not apply to the caller below since the test is Windows-specific. See my answer regarding 'attrib' below.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> + def test_execute_windows_tool(self):<br>
> + if (sys.platform != 'windows'<br>
> + and sys.platform != 'cygwin'):<br>
> + raise tests.TestSkipped('Platform does not have Windows tools.')<br>
<br>
TestSkipped is rather generic. We usually use UnavailableFeature (via<br>
our requireFeature code) to indicate platform differences. Though the<br>
"feature" in this case is a deficiency, bringing new meaning to the<br>
phrase "it's not a bug, it's a feature".<br>
</blockquote><div><br>Is there an agreement between you and Alexander?<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> + output = StringIO()<br>
> + tree = self.make_branch_and_tree('tree')<br>
> + self.build_tree_contents([('tree/file', 'content')])<br>
> + tree.add('file', 'file-id')<br>
> + tree.commit('old tree', timestamp=0)<br>
<br>
^^^ The timestamp value doesn't seem to be required here.<br>
</blockquote><div><br>OK, I can remove that.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> + tree.lock_read()<br>
> + self.addCleanup(tree.unlock)<br>
> + diff_obj = DiffFromTool(['python', '-c',<br>
> + 'print "%(old_path)s %(new_path)s"'],<br>
> + tree, tree, output)<br>
> + diff_obj._prepare_files('file-id', 'file', 'file')<br>
> + proc = subprocess.Popen(['attrib', 'old/file'],<br>
> + stdout=subprocess.PIPE,<br>
> + cwd=diff_obj._root)<br>
<br>
^^^ err, what are you doing with attrib here? We do not normally use<br>
subprocesses in our test suite.<br></blockquote><div><br>Verifying that a tool (attrib) in the host OS (Windows) that understands mixed paths (with '/' rather than '\') can read the files on both sides of the diff.<br>
<br>I'm using a subprocess because the feature I'm testing is support for diff subprocesses in the host OS.<br><br>Matt<br></div></div>