Re: [PATCH 1/5] cmd/tests: add more tests for command line parsing
On Tue, Aug 30, 2011 at 00:34, Martin Wilck <mwilck(a)arcor.de> wrote:
This patch adds some tests for command line parsing and treatment of quotes and special characters in the command line and in program names and parameters. The .exp file was created using Win XP. Follow-up patches will fix the marked TODOs. --- programs/cmd/tests/rsrc.rc | 6 + programs/cmd/tests/test_parsing.cmd | 158 +++++++++++++++++++++++++++++++ programs/cmd/tests/test_parsing.cmd.exp | 77 +++++++++++++++ 3 files changed, 241 insertions(+), 0 deletions(-) create mode 100644 programs/cmd/tests/test_parsing.cmd create mode 100644 programs/cmd/tests/test_parsing.cmd.exp
Some of those tests are already present in the test_builtins.cmd* (e.g. the spacing tests) You should probably integrated your tests in there: that makes it easier to spot regressions and helps prevent tests duplication Also, see http://www.winehq.org/pipermail/wine-devel/2011-August/091817.html Frédéric Delanoy
Hi Frédéric,
Some of those tests are already present in the test_builtins.cmd* (e.g. the spacing tests) You should probably integrated your tests in there: that makes it easier to spot regressions and helps prevent tests duplication
I can certainly do that if you prefer (and I will), but I don't agree that having all tests in a single big file is an advantage. Splitting the unit tests over multiple files would make it much easier to relate output lines to script lines and do focused work on certain areas. For anyone who doesn't work on it regularly, test_builtins.cmd is a nightmare.
Also, see http://www.winehq.org/pipermail/wine-devel/2011-August/091817.html
I'm not sure why you're telling me that. My patches aren't about performance but correctness. They won't affect performance negatively, either. Regards, Martin PS: any idea why my patches didn't make it to wine-patches?
2011/8/30 Martin Wilck <mwilck(a)arcor.de>:
Hi Frédéric,
Some of those tests are already present in the test_builtins.cmd* (e.g. the spacing tests) You should probably integrated your tests in there: that makes it easier to spot regressions and helps prevent tests duplication
I can certainly do that if you prefer (and I will), but I don't agree that having all tests in a single big file is an advantage. Splitting the unit tests over multiple files would make it much easier to relate output lines to script lines and do focused work on certain areas.
Cmd test suite would probably eventually benefit from this on the long term, but it's still very incomplete in a number of areas (thanks for helping improve it BTW). IMHO we should probably split it only when it's reasonably complete, where we'd have a broader view. (it also makes viewing regression easier for the devs since the parser is rather fragile, and everything is a bit intertwined) Also, test_parsing name is probably a bit too generic (not that test_builtins isn't ;) ) But you can still submit it in the meantime, maybe AJ will accept it. Just make sure there's no duplication
For anyone who doesn't work on it regularly, test_builtins.cmd is a nightmare.
Just look at cmd parser code if you really want to be afraid ;) I'm a bit responsible for this, but the current parser has numerous shortcomings/bugs that sometimes force to use ugly workarounds. Also, when you have to make tests work on NT4->2k8 *and* wine at *all* times, you sometimes have to make your hands dirty. (e.g. a restriction is that you need the same # of lines of the processed cmd file, and the expected file, on all windowses and wine. There's no support for skip() or win_skip() either like is available in the other regular C unit tests. I guess that eating one's own dogfood...) Just ask if you don't understand the @keywords(a). Doc is a bit lacking; I might add a quick wiki help page if desired. Basically the most important ones are @todo_wine@ and @or_broken@ (respectively to indicate the test fails on wine, or have alternate results in other windowses) Wine testbot can also help (https://testbot.winehq.org/index.pl)
Also, see http://www.winehq.org/pipermail/wine-devel/2011-August/091817.html
I'm not sure why you're telling me that. Well the mentioned link was about splitting tests ("Cmd tests timeout and splitting tests") The discussion thread was about splitting the test file into distinct subfiles, such as 1 per builtin or so do a timeout on some linux machines. (Cmd's testsuite run in a matter of seconds on windowses, but much longer on distinct linux/bsd test machines)
My patches aren't about performance but correctness. They won't affect performance negatively, either. Of course not, since it's only in the test proc
Regards, Martin
Nice job by the way. Frédéric
PS: any idea why my patches didn't make it to wine-patches?
Did you correctly register via http://www.winehq.org/mailman/listinfo/wine-patches?
IMHO we should probably split it only when it's reasonably complete, where we'd have a broader view.
Well, it's up to you. I just made an attempt to merge my stuff into test_builtins and gave up after 2 hours :-(
Also, test_parsing name is probably a bit too generic (not that test_builtins isn't ;) )
But you can still submit it in the meantime, maybe AJ will accept it. Just make sure there's no duplication
AFAICT there is no real duplication. 4 TODOs in the current test_builtins are removed by the last of my patches, so there is some overlap, but I wouldn't call it a duplicate.
Also, when you have to make tests work on NT4->2k8 *and* wine at *all* times, you sometimes have to make your hands dirty.
I tested my stuff only on XP. Perhaps someone can help me out testing other native versions.
Just ask if you don't understand the @keywords(a). Doc is a bit lacking; I might add a quick wiki help page if desired.
Yes, that'd be helpful (IMO just a README file in the tests/ directory would be even better). It took me a while to get started with the cmd testing code.
Did you correctly register via http://www.winehq.org/mailman/listinfo/wine-patches?
I was hoping this was an open ML (and last time tried it was). OK, I'll subscribe and re-submit.
Nice job by the way.
Thanks a lot, it seems that you took on a big task with cmd, good luck! Martin
2011/8/31 Martin Wilck <mwilck(a)arcor.de>:
IMHO we should probably split it only when it's reasonably complete, where we'd have a broader view.
Well, it's up to you. I just made an attempt to merge my stuff into test_builtins and gave up after 2 hours :-(
Also, test_parsing name is probably a bit too generic (not that test_builtins isn't ;) )
But you can still submit it in the meantime, maybe AJ will accept it. Just make sure there's no duplication
AFAICT there is no real duplication. 4 TODOs in the current test_builtins are removed by the last of my patches, so there is some overlap, but I wouldn't call it a duplicate.
Fair enough.
Also, when you have to make tests work on NT4->2k8 *and* wine at *all* times, you sometimes have to make your hands dirty.
I tested my stuff only on XP. Perhaps someone can help me out testing other native versions.
You should subscribe to testbot. It has a shitload of available VMs to tests your patches against. Actually, my current test procedure is the following: 1. Write a passing test/expected result on a reasonably old win version (e.g. 2k or XP) 2. Submit to testbot (http://wiki.winehq.org/WineTestBot) to check it works on win versions (sometimes it fails on NT4 or other win version due to random bugs) If it fails, iterate until it passes on windows 3. Check if it works on wine (make testclean && make test) If it works, you're done. Else, make changes and goto 2
Just ask if you don't understand the @keywords(a). Doc is a bit lacking; I might add a quick wiki help page if desired.
Yes, that'd be helpful (IMO just a README file in the tests/ directory would be even better). It took me a while to get started with the cmd testing code.
Yeah me too. Wine's learning curve is steep. Did you read http://www.winehq.org/docs/winedev-guide/index ? It's not entirely relevant to cmd's tests but is a good starting point. I'll write a patch adding a README. There's also IRC #winehackers channel on Freenode.
Did you correctly register via http://www.winehq.org/mailman/listinfo/wine-patches?
I was hoping this was an open ML (and last time tried it was). OK, I'll subscribe and re-submit.
Think "spam". Also you might want to submit smaller patches, it helps with regression testing (and eases code review by the committer)
Nice job by the way.
Thanks a lot, it seems that you took on a big task with cmd, good luck!
Martin
Frédéric
2011/8/31 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
2011/8/31 Martin Wilck <mwilck(a)arcor.de>:
Just ask if you don't understand the @keywords(a). Doc is a bit lacking; I might add a quick wiki help page if desired.
Yes, that'd be helpful (IMO just a README file in the tests/ directory would be even better). It took me a while to get started with the cmd testing code.
Submitted in http://source.winehq.org/patches/data/78195 but Alexandre prefers a wiki page so I'll convert it. Frédéric
2011/8/31 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
2011/8/31 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
2011/8/31 Martin Wilck <mwilck(a)arcor.de>:
Just ask if you don't understand the @keywords(a). Doc is a bit lacking; I might add a quick wiki help page if desired.
Yes, that'd be helpful (IMO just a README file in the tests/ directory would be even better). It took me a while to get started with the cmd testing code.
Submitted in http://source.winehq.org/patches/data/78195 but Alexandre prefers a wiki page so I'll convert it.
Available on http://wiki.winehq.org/CmdConformanceTests
participants (2)
-
Frédéric Delanoy -
Martin Wilck