Re: cmd: Add test for %~dp0 expansion (bug 21382)
On Fri, Feb 12, 2010 at 1:55 AM, Alexandre Julliard julli...@winehq.org wrote: Please don't create new files for every small test, this will become unmanageable very quickly. Unless there are reasons to split them, everything should go in a single .cmd file. The current cmd test framework is not currently clever about resyncing after an error, so any missing or extra line in a test's output can cascade and cause lots of false reports. How about one file for each command recognized by cmd? So there'd be test_if.cmd (which might cover else as well), test_goto.cmd, etc. That feels more manageable to a test writer, I suspect. The only problem is that pesky .rc file; it'd be nice to autogenerate it. I can submit a patch to do that, but you rejected that last time, probably because dependencies were not handled perfectly. Would a patch that generated the .rc file be accepted if it got dependencies perfect? +echo Begin test for bug 21382 +rem On old wine, ~dp0 was always current directory rather than directory containing batch file ... +echo End test for bug 21382 Also don't use bug numbers, add some meaningful message instead. The meaningful message is already there: +rem On old wine, ~dp0 was always current directory rather than directory containing batch file The code must be understandable without reference to bugzilla. Agreed. For the day when our test files get long (some commands are going to have lots of options), it will make the result parser's job a lot easier to have unique-ish begin and end tags in the output. Bug ID is as good as anything for a unique ID when it's available. Maybe I can make that clearer, e.g. echo begin bug21382 rem meaningful description of this test goes here ... echo end bug21382 OK? Apologies for the long email. irc would be better for this kind of disagreement, but I didn't see you online just now. - Dan
Re: cmd: Add test for %~dp0 expansion (bug 21382)
On 2/13/10 4:49 PM, Dan Kegel wrote: On Fri, Feb 12, 2010 at 1:55 AM, Alexandre Julliardjulli...@winehq.org wrote: Please don't create new files for every small test, this will become unmanageable very quickly. Unless there are reasons to split them, everything should go in a single .cmd file. The current cmd test framework is not currently clever about resyncing after an error, so any missing or extra line in a test's output can cascade and cause lots of false reports. It's easy to fix. We can extend result templates to handle it just like todo_space. However, I don't think we want to make it more complicated than needed. That is, we can extend it when we will find a real problem that needs to be addressed. That would be something worth testing and hard enough to fix in cmd. How about one file for each command recognized by cmd? So there'd be test_if.cmd (which might cover else as well), test_goto.cmd, etc. That feels more manageable to a test writer, I suspect. That's how I meant it to be when I called the first test test_echo.cmd. It does not need to be strictly by-command rule, but just set of logically connected tests. The only problem is that pesky .rc file; it'd be nice to autogenerate it. I can submit a patch to do that, but you rejected that last time, probably because dependencies were not handled perfectly. Would a patch that generated the .rc file be accepted if it got dependencies perfect? I don't see much need for this. We won't have that many files and adding them is not too hard. +echo Begin test for bug 21382 +rem On old wine, ~dp0 was always current directory rather than directory containing batch file ... +echo End test for bug 21382 Also don't use bug numbers, add some meaningful message instead. The meaningful message is already there: +rem On old wine, ~dp0 was always current directory rather than directory containing batch file 'On old wine' doesn't sound right. You want to show the correct behavior, not describe how broken wine was in the past. The code must be understandable without reference to bugzilla. Agreed. For the day when our test files get long (some commands are going to have lots of options), it will make the result parser's job a lot easier to have unique-ish begin and end tags in the output. Bug ID is as good as anything for a unique ID when it's available. Maybe I can make that clearer, e.g. echo begin bug21382 rem meaningful description of this test goes here ... echo end bug21382 OK? I don't see how bug numbers would be helpful. All you want to do is to make sure that cmd behavior is correct. It doesn't matter how broken it was in the past. If your change breaks a test, it's easy to find how and why by reading error message and looking at tests. If you really want to find the reason why the test was added, that's what git blame is for. Jacek
Re: cmd: Add test for %~dp0 expansion (bug 21382)
Dan Kegel d...@kegel.com writes: The current cmd test framework is not currently clever about resyncing after an error, so any missing or extra line in a test's output can cascade and cause lots of false reports. How about one file for each command recognized by cmd? So there'd be test_if.cmd (which might cover else as well), test_goto.cmd, etc. That feels more manageable to a test writer, I suspect. The only problem is that pesky .rc file; it'd be nice to autogenerate it. I can submit a patch to do that, but you rejected that last time, probably because dependencies were not handled perfectly. Would a patch that generated the .rc file be accepted if it got dependencies perfect? Maybe, but you still need to maintain the list of files somewhere. And in any case it doesn't change the fact that the number of files must be kept small. If the test program is not clever enough to manage large files it should be fixed. For the day when our test files get long (some commands are going to have lots of options), it will make the result parser's job a lot easier to have unique-ish begin and end tags in the output. Bug ID is as good as anything for a unique ID when it's available. Maybe I can make that clearer, e.g. echo begin bug21382 rem meaningful description of this test goes here ... echo end bug21382 OK? No, that's just noise. Simply output the description instead of outputting a random id and hiding the description in a comment. -- Alexandre Julliard julli...@winehq.org
Re: cmd: Add test for %~dp0 expansion (bug 21382)
On Sat, Feb 13, 2010 at 9:02 AM, Alexandre Julliard julli...@winehq.org wrote: the number of files must be kept small. Is one per built-in command of wcmd too many? - Dan
Re: cmd: Add test for %~dp0 expansion (bug 21382)
Dan Kegel d...@kegel.com writes: On Sat, Feb 13, 2010 at 9:02 AM, Alexandre Julliard julli...@winehq.org wrote: the number of files must be kept small. Is one per built-in command of wcmd too many? At this point, yes. Once we have a thousand-line test for each command, then it may be a reasonable split. For now, just put everything into one file until it becomes so large than splitting is warranted. -- Alexandre Julliard julli...@winehq.org
Re: cmd: Add test for %~dp0 expansion (bug 21382)
Dan Kegel d...@kegel.com writes: --- /dev/null +++ b/programs/cmd/tests/test_dp0.cmd @@ -0,0 +1,10 @@ +...@echo off +echo Begin test for bug 21382 +rem On old wine, ~dp0 was always current directory rather than directory containing batch file +echo %~dp0 +mkdir dummydir +cd dummydir +echo %~dp0 +cd .. +rmdir dummydir +echo End test for bug 21382 diff --git a/programs/cmd/tests/test_dp0.cmd.out b/programs/cmd/tests/test_dp0.cmd.out new file mode 100644 index 000..8c38153 --- /dev/null +++ b/programs/cmd/tests/test_dp0.cmd.out @@ -0,0 +1,4 @@ +Begin test for bug 21382 +...@pwd@\ +...@pwd@\ +End test for bug 21382 Please don't create new files for every small test, this will become unmanageable very quickly. Unless there are reasons to split them, everything should go in a single .cmd file. Also don't use bug numbers, add some meaningful message instead. The code must be understandable without reference to bugzilla. -- Alexandre Julliard julli...@winehq.org
