For the moment, the results only go to a web page, http://kegel.com/wine/patchwatcher/results/ Most of the results there right now are just test messages so you can see how it will look when real patches with various problems are received.
The scripts are a bit ugly, so expect problems to linger for the next week or so while I work out the kinks.
On Sat, Aug 9, 2008 at 9:01 PM, Dan Kegel dank@kegel.com wrote:
For the moment, the results only go to a web page, http://kegel.com/wine/patchwatcher/results/ Most of the results there right now are just test messages so you can see how it will look when real patches with various problems are received.
The scripts are a bit ugly, so expect problems to linger for the next week or so while I work out the kinks.
I just noticed Ambroz actually came through with the chroot: http://article.gmane.org/gmane.comp.emulators.wine.devel/62314
I had already moved to using a separate userid, and switched to a very low-privilege way of uploading results, so my security story is probably ok for a little while, but I'll try to integrate the chroot one of these days soon for even more protection.
On Sat, Aug 9, 2008 at 9:01 PM, Dan Kegel dank@kegel.com wrote:
For the moment, the results only go to a web page, http://kegel.com/wine/patchwatcher/results/ Most of the results there right now are just test messages so you can see how it will look when real patches with various problems are received.
The scripts now run conformance tests and report regressions. It's pretty rough still - I haven't compensated for flaky tests properly yet. And I'm running them on a slowish machine, so it lags by twenty minutes or so.
The source is still at http://code.google.com/p/winezeug/source/browse/#svn/trunk/patchwatcher if anyone else is curious, or wants to run it themselves.
Am Montag, den 11.08.2008, 09:45 -0700 schrieb Dan Kegel:
The scripts now run conformance tests and report regressions.
Does "Ditto, but just the new error:"<end of output> mean that there are no new errors?
Regards, Michael Karcher
On Mon, Aug 11, 2008 at 10:13 AM, Michael Karcher wine@mkarcher.dialup.fu-berlin.de wrote:
Am Montag, den 11.08.2008, 09:45 -0700 schrieb Dan Kegel:
The scripts now run conformance tests and report regressions.
Does "Ditto, but just the new error:"<end of output> mean that there are no new errors?
Yes. Sorry, I'll tidy that up tonight, it really should say "no regressions found".
Hi,
I have one more concern. Its regarding running of tests. When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming. What will happen if we have patch barrage, like once when alexander comes from vacation. How much time will that take for verification of the entire patch tree submitted then.
Just my $0.02
Thanks, VJ
On Mon, Aug 11, 2008 at 3:05 PM, Dan Kegel dank@kegel.com wrote:
On Mon, Aug 11, 2008 at 10:13 AM, Michael Karcher wine@mkarcher.dialup.fu-berlin.de wrote:
Am Montag, den 11.08.2008, 09:45 -0700 schrieb Dan Kegel:
The scripts now run conformance tests and report regressions.
Does "Ditto, but just the new error:"<end of output> mean that there are no new errors?
Yes. Sorry, I'll tidy that up tonight, it really should say "no regressions found".
On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju infyquest@gmail.com wrote:
When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming.
True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it.
What will happen if we have patch barrage, like once when alexander comes from vacation.
It'll fall behind some. If need be, I can run it on a really fast machine. - dan
On Mon, Aug 11, 2008 at 7:42 PM, Dan Kegel dank@kegel.com wrote:
On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju infyquest@gmail.com wrote:
When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming.
True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it.
Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher. Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here)
What will happen if we have patch barrage, like once when alexander comes from vacation.
It'll fall behind some. If need be, I can run it on a really fast machine.
It would better if we have a parallelized version of the tests also run on a fast m/c. Also can you improve the messages. If there are errors, Its possible to only show the test data that failed rather than the complete test run. Also put it in a public repository with you as sole commiter. So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) )
---- VJ
- dan
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher.
It's running on a 1GHz single core machine right now. I'll probably put it on something rather faster.
Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here)
No. Probably should, but I'm not.
It would better if we have a parallelized version of the tests also run on a fast m/c.
I do have a patch that enables parallel execution of conformance tests, I hope Alexandre accepts it. That will help on multicore systems. Beyond that, I could fairly easily use multiple machines, e.g. assign all patches to machines based on md5sum.
Also can you improve the messages.
Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure.
If there are errors, Its possible to only show the test data that failed rather than the complete test run.
Yes.
Also put it in a public repository with you as sole commiter.
Already there, see http://code.google.com/p/winezeug/
So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) )
Please do. - Dan
On Mon, Aug 11, 2008 at 8:34 PM, Dan Kegel dank@kegel.com wrote:
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher.
It's running on a 1GHz single core machine right now. I'll probably put it on something rather faster.
Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here)
No. Probably should, but I'm not.
It would better if we have a parallelized version of the tests also run on a fast m/c.
I do have a patch that enables parallel execution of conformance tests, I hope Alexandre accepts it. That will help on multicore systems. Beyond that, I could fairly easily use multiple machines, e.g. assign all patches to machines based on md5sum.
Also can you improve the messages.
Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure.
If there are errors, Its possible to only show the test data that failed rather than the complete test run.
Yes.
Also put it in a public repository with you as sole commiter.
Already there, see http://code.google.com/p/winezeug/
So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) )
Please do.
- Dan
Dan, how are you handling the case when Alexandre floods the list with commits? One way I can think of to avoid those patches is to ignore any patches emailed by Alexandre but aren't written by him. Its not the worst thing in the world if the script doesn't skip commits, just some wasted perf.
Zachary Goldberg zgold550@gmail.com wrote:
[much quoted text]
Please trim the quotes down a bit when you reply...
Dan, how are you handling the case when Alexandre floods the list with commits?
See refresh_tree(), http://code.google.com/p/winezeug/source/browse/trunk/patchwatcher/patchwatc... After processing each patch (or patch series), patchwatcher check for git commits. If it finds any, it recomputes the expected test failures. This is slow, but it only happens once or twice a day, so it's ok.
One way I can think of to avoid those patches is to ignore any patches emailed by Alexandre but aren't written by him.
Alexandre never emails any patches at all, he just commits them directly. So patchwatcher will never give any feedback on his changes. - Dan
On Mon, Aug 11, 2008 at 8:34 PM, Dan Kegel dank@kegel.com wrote:
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Ok I was expressing my concern as it took around 2-3hrs to see my patch in the patchwatcher.
It's running on a 1GHz single core machine right now. I'll probably put it on something rather faster.
Something around 2GHz would be ok
Also as you you running the wine tests all for each patch are you cleaning the .wine directory ( I am bit confused here)
No. Probably should, but I'm not.
I think you should do that, if there are some changes that involve wineserver and some change to registry.
It would better if we have a parallelized version of the tests also run on a fast m/c.
I do have a patch that enables parallel execution of conformance tests, I hope Alexandre accepts it. That will help on multicore systems. Beyond that, I could fairly easily use multiple machines, e.g. assign all patches to machines based on md5sum.
Ok, it would be interesting how parallelization could affect wine :)
Also can you improve the messages.
Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure.
Also add Yellow for ignored patches. For ignored patches /i would like to add a second pass, when have to check if the patch is generated by git or not if not patch is being ignored now, for that we need to process the patch to see whether we can extract some info to send the correct patch level.
If there are errors, Its possible to only show the test data that failed rather than the complete test run.
Yes.
Also put it in a public repository with you as sole commiter.
Already there, see http://code.google.com/p/winezeug/
Thanks
So If we have any suggestions/improvements, can mail you with the changes (We will not flood ur mail box ;) )
Please do.
I will look into it now ---- VJ
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Also add Yellow for ignored patches.
Let me think on that a bit. Probably.
For ignored patches /i would like to add a second pass, when have to check if the patch is generated by git or not if not patch is being ignored now, for that we need to process the patch to see whether we can extract some info to send the correct patch level.
Yes, I plan to add a little code to try to guess the right argument to -p. - Dan
On Mon, Aug 11, 2008 at 11:02 PM, Dan Kegel dank@kegel.com wrote:
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Also add Yellow for ignored patches.
Let me think on that a bit. Probably.
For ignored patches /i would like to add a second pass, when have to check if the patch is generated by git or not if not patch is being ignored now, for that we need to process the patch to see whether we can extract some info to send the correct patch level.
Yes, I plan to add a little code to try to guess the right argument to -p.
Ahh... I forgot how to handle dependent patches, if they are not in a patch series Sorry, I always seem to get all kinda of weird ideas about how things can go wrong ---- VJ
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Ahh... I forgot how to handle dependent patches, if they are not in a patch series
I don't know if there's a good way to handle those. Maybe just encourage people not to send them :-)
On Mon, Aug 11, 2008 at 11:52 PM, Dan Kegel dank@kegel.com wrote:
Vijay Kiran Kamuju infyquest@gmail.com wrote:
Ahh... I forgot how to handle dependent patches, if they are not in a patch series
I don't know if there's a good way to handle those. Maybe just encourage people not to send them :-)
Policy is that all patches should be independent, no?
(Sorry about big quoting, gmail just compresses it so nicely)
"Zachary Goldberg" zgold550@gmail.com wrote:
Policy is that all patches should be independent, no?
There is no such a policy. Dependent patches are marked as 1/xx, 2/xx, ... xx/xx.
Dmitry Timoshkov dmitry@codeweavers.com wrote:
"Zachary Goldberg" zgold550@gmail.com wrote:
Policy is that all patches should be independent, no?
There is no such a policy. Dependent patches are marked as 1/xx, 2/xx, ... xx/xx.
That's a patch series, and patchwatcher handles that ok.
There's another kind of dependent patch, where somebody says "This requires Harold's patch from yesterday". Patchwatcher probably isn't going to handle that ever.
"Dan Kegel" dank@kegel.com wrote:
That's a patch series, and patchwatcher handles that ok.
There's another kind of dependent patch, where somebody says "This requires Harold's patch from yesterday". Patchwatcher probably isn't going to handle that ever.
Well, that happens not that often, so this can be safely ignored for now I think.
On Mon, 2008-08-11 at 22:24 -0700, Dan Kegel wrote:
Dmitry Timoshkov dmitry@codeweavers.com wrote:
"Zachary Goldberg" zgold550@gmail.com wrote:
Policy is that all patches should be independent, no?
There is no such a policy. Dependent patches are marked as 1/xx, 2/xx, ... xx/xx.
That's a patch series, and patchwatcher handles that ok.
There's another kind of dependent patch, where somebody says "This requires Harold's patch from yesterday". Patchwatcher probably isn't going to handle that ever.
What about checking for a string in the email like "patchwatchignore"; if for some reason the patch is known to cause a failure the e-mail might read like: patchwatchignore This depends on Harald's patch from yesterday... [patch]
This also might be useful for patches which update comments or README files, etc.
On Tue, Aug 12, 2008 at 1:39 PM, Adam Petaccia adam@tpetaccia.com wrote:
What about checking for a string in the email like "patchwatchignore"; if for some reason the patch is known to cause a failure the e-mail might read like: patchwatchignore This depends on Harald's patch from yesterday... [patch]
If patchwatcher becomes a gatekeeper, we will add a bypass like that.
This also might be useful for patches which update comments or README files, etc.
It's a lot easier to let the hardware run the tests, and as long as the hardware is fast enough to not get backlogged, the payoff for using the bypass for doc-only changes is small.
Am Montag, den 11.08.2008, 17:34 -0700 schrieb Dan Kegel:
Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure.
I dislike the implementation, while I like the idea. You now have:
a:visited { color: #FF0000; } .fail { background color: #ff5050; }
At least on my laptop display, #ff5050 on #ff0000 is quite hard to read.
Regards, Michael Karcher
Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure.
I dislike the implementation, while I like the idea. You now have:
a:visited { color: #FF0000; } .fail { background color: #ff5050; }
At least on my laptop display, #ff5050 on #ff0000 is quite hard to read.
I think making the a:visited link for pass or fail to be balck is good enough. I think this should do the trick. .pass a:visited { color: #000000; } .fail a:visited { color: #000000; }
--- VJ
Michael Karcher wine@mkarcher.dialup.fu-berlin.de wrote:
Yes. I already changed the success message to make more sense, and added background colors of green and red for success and failure.
I dislike the implementation, while I like the idea. You now have:
a:visited { color: #FF0000; } .fail { background color: #ff5050; }
At least on my laptop display, #ff5050 on #ff0000 is quite hard to read.
Yeah, I know. I fiddled with the colors for a while, but not very effectively. I'm partly color-blind, and am not really the best person to work on the look of the reports page. If somebody else would like to get the colors right, I'd gladly accept patches.
Would using black for foreground uniformly be more acceptable?
Am Dienstag, den 12.08.2008, 08:26 -0700 schrieb Dan Kegel:
Yeah, I know. I fiddled with the colors for a while, but not very effectively. I'm partly color-blind, and am not really the best person to work on the look of the reports page. If somebody else would like to get the colors right, I'd gladly accept patches.
Would using black for foreground uniformly be more acceptable?
Looks better now.
UI suggestion: If tests fail, format the status column like this instead:
<a href="results/1.diff">5 regressions</a> in <a href="results/1.log">tests</a>
where results/1.diff just contains the diff output that is currently appended to the log file.
I have the impression that the whole table just is too wide. For me, real name would definitely be shorter than e-mail address, but using Yet it would be even greater if my patches would get green. They seem to fail afoul flaky tests, but you are already aware of that problem. adresses might be advantageous because they are (a) unique and (b) definitely ASCII charset. Also subjects might be shortened by removing PATCH (I am sorry, but I didn't get git to number the series and not output "PATCH", any hints welcome!) and cutting the subject after 40 characters.
Still, besides all the critique: Great work!
Regards, Michael Karcher
PS: Yet patchwatcher would be even greater if my patches would get green. They seem to fail afoul flaky tests, but you are already aware of that problem.
Dan Kegel wrote:
On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju infyquest@gmail.com wrote:
When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming.
True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it.
I'd argue that testing just the affected dll is correct. What about things like patches to ntdll/kernel32/advapi32 (and the likes). They could influence far more tests then just the ones for it's own dll.
On Tue, Aug 12, 2008 at 1:01 AM, Paul Vriens paul.vriens.wine@gmail.com wrote:
Dan Kegel wrote:
On Mon, Aug 11, 2008 at 4:31 PM, Vijay Kiran Kamuju infyquest@gmail.com wrote:
When running tests for the patch, I think we should just run the tests of the dlls that are affected direct;y or indirectly by that change. its running the tests for entire wine, which is very time consuming.
True, but hey, it was easier to code. And getting anything like this working at all is pretty hard. Figuring out which tests a give patch affects is an extra challenge I'd rather not face just now. Once it's up and working well we can refine it.
I'd argue that testing just the affected dll is correct. What about things like patches to ntdll/kernel32/advapi32 (and the likes). They could influence far more tests then just the ones for it's own dll.
Which is an argument for why you should test the entire tree for each patch. If a patch to ntdll makes tests in another dll fail, the patch should be rejected just as if the ntdll tests failed. This is how Julliard's work flow goes, so the test bot should do the same.
-
Adam Petaccia -
Dan Kegel -
Dmitry Timoshkov -
James Hawkins -
Michael Karcher -
Paul Vriens -
Vijay Kiran Kamuju -
Zachary Goldberg