[chromium-dev] Re: Platform-dependent form control rendering in layout tests
Not yet, patches are welcomed. (Note that it requires implementing the support to run custom tests, which is already implemented client-side but not server-side yet) Also, we have 4 W2k3 servers with a lot of buildbot VM images that can be used when you need a specific OS for testing. M-A On Mon, Jan 5, 2009 at 1:32 AM, Adam Barth aba...@chromium.org wrote: Can you grab the pixel results from the try server? On Sun, Jan 4, 2009 at 10:14 PM, Brett Wilson bre...@chromium.org wrote: I just upgraded my dev box to one of the new quad-cores and put Vista 64 on it so I can use more memory for the disk cache. (Note: XP64 isn't one of the standard corp OS images.) I suspect more people will want to start doing this (I'm still doing tests, will send out the results). I just realized that I have a problem now: I have no way to run webkit pixel tests since the form control reference images all require XP's Luna theme. I have three boxes: Vista, Mac, and Linux, so it's not reasonable to get another one. It also seems impossible to require everybody use XP forever. WebKit solves this by putting the layout test results of each build in a public place on the buildbot (I don't think ours are, are they?) and when you change something that has a reference that isn't for your system, you just grab it off there, double-check that it's correct, and check it in. However, this requires more tolerance of test redness than we have had so far. The other option is to use platform-independent form control rendering. This actually isn't too hard. WebKit already has form control rendering for when you have funky CSS styles applied (think when you set border-top:2px solid red; on a text fiend) and we can enable this easily enough in our theme control by returning the correct values (maybe there are a few edge cases that need cleaning up). This has the advantage of letting people running XP, Vista or either of those with the classic theme run the layout tests in a meaningful way. It has the disadvantage that none of our native theme rendering will be covered by pixel tests, which could lead to regressions. I'm not sure which approach is better. Does anybody have any thoughts? Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: webkit/port is moving into third_party/WebKit/WebCore
On Mon, Jan 5, 2009 at 8:30 AM, Mike Pinkerton pinker...@chromium.org wrote: Awesome! Now that so much of our code is in the webkit tree, is there a (public) wiki page describing the steps necessary to make changes to anything within third_party/WebKit/WebCore? i.e, does everything have to go upstream, are there any circumstances when forking is allowed on a vendor branch, when should DEPS be rolled in this new world, etc? Seems like things have changed so quickly over the last few months (in a good way), it might be helpful to lay out the current state of things for everyone to re-baseline their knowledge. Currently our port is not upstream, it is just checked into our local copy of WebKit where they will soon be checked in upstream. So you can continue to change our port when you want, although you will have to roll webkit deps by updating src/DEPS to pull your change into Chrome. Once we check in upstream, all changes should be checked in upstream, and they will be pulled in the next day's WebKit merge. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: building with shared libraries
Mozilla has this flexibility (for all platforms) and it's a very nice feature in their build system. I think ultimately we should aspire to do this on mac and windows too (hopefully when we're all unified on [insert newfangled build system here]). On Sun, Dec 28, 2008 at 11:47 AM, skana...@gmail.com skana...@gmail.com wrote: Just added 'SHARED=1': http://code.google.com/p/chromium/wiki/ChromiumSoftwareConstructionSystem On Dec 26, 6:24 am, Evan Martin e...@chromium.org wrote: I've checked in some changes that make it so you can build (on Linux) with shared libraries. This significantly reduces link times and overall build times at the cost of slower code and slower startup (i.e., you shouldn't use it for user-facing builds). == Background Chromium has historically built as one huge shared object (.dll/.so), which means the linker can make more intelligent decisions (like eliminating dead code across modules) at the cost of making link times significantly slower. For example, a debug build of test_shell on linux is currently 400+ megabytes (!) and part of the way scons builds involves copying that file once upon successful linking (!!). A shared-library version of test_shell produces a 232kb executable along with 26 .so files. In theory, if you don't touch code within one of the chrome submodules, then its .so doesn't need to be relinked. == How to use it $ normal_hammer_command_line SHARED=1 Source files used in libraries should compile to .os rather than .o (they're different outputs -- e.g. position-independent code with -fPIC). If you have .so files in Hammer/dbg/lib/ , it appears to always produce shared executables regardless of your SHARED setting (?). I need to investigate further why. You can rm Hammer/dbg/lib/*.so to stop that for now. == Link time numbers Editing one test_shell file, rebuilding: 46s. Null build after that: 40s. This implies that the compile+link+copy time of test_shell was 6 seconds and the other 40 is scons-related overhead (including stat()ing a bunch of files, etc.). For comparison, linking test_shell statically takes 124s on my laptop, with 42 second null build time. So it saves about 78 seconds to dynamically link, or it's roughly 13 times faster. (Note that above numbers are with the slow standard binutils linker, rather than gold, and with my slow laptop, so your times may vary.) == Grungy details The scons build scripts do most of the work, including setting RPATH on the resulting binary, so you don't need to mess with LD_LIBRARY_PATH. We now use ChromeLibrary (rather than ChromeStaticLibrary) for all build commands, and should also use it in the future. Stuff that always needs to be shared (i.e. plugins) can continue using ChromeSharedLibrary. == Fixing modules that are missing symbols when built shared The dynamic linker refuses to build shared libraries that are missing symbols. It turns out in our code since we have traditionally statically linked everything, we've been ok with missing symbols as long as those functions/data are never referenced. I had to make some changes to bring in or stub out missing bits of code; in a static build they should be eliminated by the linker as before. I expect the shared build to occasionally break in the future due to this difference. It might be worth making a builder for it, or just switching the Linux debug build to it. Another difference with shared code is dependency chains matter more. googleurl's unittest uses libbase, which officially depends on libevent despite the googleurl unittest not making use of libevent. So for a static build, it links fine, but for a dynamic build you'll get missing symbol errors when linking. The fix is to change anybody who uses libbase to pull in libevent as well (on platforms that use libevent). The proper way to do this is to modify using_base.scons to pull in libevent, and then change anyone who uses libbase to use using_base.scons instead of referring to base directly. Check out build/googleurl_unittests.scons and build/using_googleurl.scons for an example. -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] HTTP BASIC/DIGEST BUG - Has it been fixed yet?
Excuse me? But how long will it take to fix this bug? I just updated my local copy of chrome and this bug still exist. I hate to sound angry but this is ridiculous. I can not recommend nor even bother to discuss Chrome to my customers as an alternative browser as long as it fails to support the most inherent and most basic fundamental HTTP standard - BASIC/DIGEST authentication, just like the rest of the majority browsers do. I know Google is trying to push Chrome as the alternative browser, but lacking proper support for HTTP authentication will continue to dog it and bad mouth it. So please, its been months since this has been reported. Thanks Hector Santos, CTO http://www.santronics.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: HTTP BASIC/DIGEST BUG - Has it been fixed yet?
It would help if you link to the bug in the bug tracker. I believe this message is in reference to http://code.google.com/p/chromium/issues/detail?id=1629 which has been marked fixed. On Mon, Jan 5, 2009 at 11:53 AM, HLS sant9...@gmail.com wrote: Excuse me? But how long will it take to fix this bug? I just updated my local copy of chrome and this bug still exist. I hate to sound angry but this is ridiculous. I can not recommend nor even bother to discuss Chrome to my customers as an alternative browser as long as it fails to support the most inherent and most basic fundamental HTTP standard - BASIC/DIGEST authentication, just like the rest of the majority browsers do. I know Google is trying to push Chrome as the alternative browser, but lacking proper support for HTTP authentication will continue to dog it and bad mouth it. So please, its been months since this has been reported. Thanks Hector Santos, CTO http://www.santronics.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: HTTP BASIC/DIGEST BUG - Has it been fixed yet?
Hi, Hector, Have you filed a bug to be specific about which problem you're having? There is a reasonable chance we're working on it, but let's be specific about what the problem is. If you can provide a repro case, that would be great. Also, we're working on some significant changes to the HTTP stack; if you run chrome.exe --new-http, that will use the new HTTP stack. When you file the bug, please let us know if it is present there too. Here are the open bugs on digest auth: http://code.google.com/p/chromium/issues/list?can=2q=digest+auth Thanks, Mike PS - As you know, the tone of your message doesn't help. On Mon, Jan 5, 2009 at 11:53 AM, HLS sant9...@gmail.com wrote: Excuse me? But how long will it take to fix this bug? I just updated my local copy of chrome and this bug still exist. I hate to sound angry but this is ridiculous. I can not recommend nor even bother to discuss Chrome to my customers as an alternative browser as long as it fails to support the most inherent and most basic fundamental HTTP standard - BASIC/DIGEST authentication, just like the rest of the majority browsers do. I know Google is trying to push Chrome as the alternative browser, but lacking proper support for HTTP authentication will continue to dog it and bad mouth it. So please, its been months since this has been reported. Thanks Hector Santos, CTO http://www.santronics.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Platform-dependent form control rendering in layout tests
I thought the plan for windows was to use interceptors like the sandbox for the system level calls (DrawThemeBackground, etc) and just mock everything. This would fix running the tests on Vista and not require a certain theme on XP. AFAIK, no one has started on this work yet. On Mon, 5 Jan 2009, Evan Martin wrote: On Sun, Jan 4, 2009 at 10:14 PM, Brett Wilson bre...@chromium.org wrote: I just upgraded my dev box to one of the new quad-cores and put Vista 64 on it so I can use more memory for the disk cache. (Note: XP64 isn't one of the standard corp OS images.) I suspect more people will want to start doing this (I'm still doing tests, will send out the results). I just realized that I have a problem now: I have no way to run webkit pixel tests since the form control reference images all require XP's Luna theme. I have three boxes: Vista, Mac, and Linux, so it's not reasonable to get another one. It also seems impossible to require everybody use XP forever. WebKit solves this by putting the layout test results of each build in a public place on the buildbot (I don't think ours are, are they?) and when you change something that has a reference that isn't for your system, you just grab it off there, double-check that it's correct, and check it in. However, this requires more tolerance of test redness than we have had so far. The other option is to use platform-independent form control rendering. This actually isn't too hard. WebKit already has form control rendering for when you have funky CSS styles applied (think when you set border-top:2px solid red; on a text fiend) and we can enable this easily enough in our theme control by returning the correct values (maybe there are a few edge cases that need cleaning up). This has the advantage of letting people running XP, Vista or either of those with the classic theme run the layout tests in a meaningful way. It has the disadvantage that none of our native theme rendering will be covered by pixel tests, which could lead to regressions. It would also let the Linux output be (nearly) shared with Windows. Currently we match Windows up to the details of antialiasing so the fuzzy differ finds them identical enough unless form controls are involved. But at least in the Linux world, we've had a ton of bugs related to form control rendering, and they appear to be subtle (e.g. taking improperly taking some transformation into account when clipping), triggering only a few layout test failures. It is important to continue testing those code paths. Does Vista not support classic-mode rendering? Clearly the solution is to require everyone to use classic. :P Slightly more seriously, you're welcome to check in Vista or OSless baselines as long as you're willing to maintain them when they break. Otherwise it'll be yet one more platform people will need to rebaseline when they fix a bug somewhere. :\ --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: webkit/port is moving into third_party/WebKit/WebCore
Awesome!
Now that so much of our code is in the webkit tree, is there a
(public) wiki page describing the steps necessary to make changes to
anything within third_party/WebKit/WebCore? i.e, does everything have
to go upstream, are there any circumstances when forking is allowed on
a vendor branch, when should DEPS be rolled in this new world, etc?
Seems like things have changed so quickly over the last few months (in
a good way), it might be helpful to lay out the current state of
things for everyone to re-baseline their knowledge.
On Wed, Dec 24, 2008 at 3:46 PM, Dimitri Glazkov dglaz...@google.com wrote:
As of Christmas Eve, we have:
webkit/port/bindings/{scripts,v8}
webkit/port/platform/image-decoders
webkit/port/platform/graphics/mac
webkit/port/DerivedSources.make
Merry Christmas and/or Festivus, everyone!
Oh, and I broke the build yesterday, and eroman fixed it. He da man.
:DG
On Tue, Dec 23, 2008 at 3:48 PM, Darin Fisher da...@chromium.org wrote:
For now, just operate the same as you would had these files still resided in
webkit/port. Just don't forget to roll DEPS :)
We'll eventually be updating the merge tracker
(http://build.chromium.org/merge) to indicate the set of files and diffs
that have yet to be upstreamed.
-Darin
On Tue, Dec 23, 2008 at 2:51 PM, Adam Barth aba...@chromium.org wrote:
I see. Can I make changes to them in third_party, or should I wait
for them to appear upstream?
Adam
On Tue, Dec 23, 2008 at 1:35 PM, Dimitri Glazkov dglaz...@google.com
wrote:
http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/WebKit/WebCore/platform/graphics/
These haven't been yet upstreamed. We just started by moving them into
our vendor branch.
:DG
On Tue, Dec 23, 2008 at 1:31 PM, Adam Barth aba...@chromium.org wrote:
I'm confused. I need to fix a bug in ImageSourceSkia.cpp, but I can't
find it
http://src.chromium.org/viewvc/chrome/trunk/src/webkit/port/platform/graphics/
or
http://trac.webkit.org/browser/trunk/WebCore/platform/graphics
Where did it go?
Adam
On Mon, Dec 22, 2008 at 8:42 PM, Darin Fisher da...@chromium.org
wrote:
OK, much of webkit/port now lives in third_party/WebKit.
All that remains is:
webkit/port/bindings/{scripts,v8}
webkit/port/bridge/chromium
webkit/port/page/chromium
webkit/port/page/inspector
webkit/port/platform/image-decoders
webkit/port/platform/mac
webkit/port/platform/graphics/mac
webkit/port/DerivedSources.make
We should be able to finish moving most of the rest of these files
tomorrow.
-Darin
On Mon, Dec 22, 2008 at 11:48 AM, Mohamed Mansour
m0.interact...@gmail.com
wrote:
I guess I will stop till your done!
On Mon, Dec 22, 2008 at 2:42 PM, Darin Fisher da...@chromium.org
wrote:
FYI
Please expect conflicts if you are trying to make changes to
webkit/port.
-Darin
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: HTTP BASIC/DIGEST BUG - Has it been fixed yet?
On Mon, Jan 5, 2009 at 12:28 PM, Evan Martin e...@chromium.org wrote: It would help if you link to the bug in the bug tracker. I believe this message is in reference to http://code.google.com/p/chromium/issues/detail?id=1629 which has been marked fixed. This bug has been fixed on the trunk of the Chromium source tree. But the fix isn't in the latest Google Chrome release yet because it is based on a branch created in November. We didn't backport the bug fix to the official branch because it's a big patch so there's a risk that we may introduce other bugs when we adapt it for the official branch, which uses a completely different HTTP stack. Hopefully the fix will appear in a Dev Channel release as part of our new HTTP stack very soon. Note: the bug is not that Chromium doesn't support HTTP BASIC and DIGEST authentication. The bug is that Chromium will only send a HTTP Authorization header after receiving a 401 Authorization Required response to the first attempt of the request. Wan-Teh Chang --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: RenderView unit tests!
Yay! Thanks a lot! On Mon, Jan 5, 2009 at 3:56 PM, Brett Wilson bre...@chromium.org wrote: Carlos created some unit testing framework for RenderWidget, and I just extended them to support RenderView. These areas have not been covered by unit tests before, and we have had some regressions as a result of our laziness. If you make changes to render_view.cc or render_widget.cc, please make sure it includes a test from now on! Please be brutal in code reviews when you see that others are making changes. If you change an area, you may end up having to write a test for things that weren't tested before (this includes porting efforts), but spending this extra time will help everybody (and all ports). Stuff you can do now with very little work: - Send any IPC message to RenderView, or call any of its methods. - Check that the RenderView sent certain IPC messages after you do something, and inspect their contents. - Load HTML and execute JS of your choosing into an offscreen RenderView. This capability covers some of the stuff we've used UI tests to cover in the past. Consider whether your UI tests can instead be expressed as a render_view_unittest, as it will be much easier to write, debug, and should be much less flaky (you don't need 3 levels of automation proxy messages just to do something simple). Don't forget about test_shell_tests if you're only testing parts of the port or glue and don't need render_view. Thanks, Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Proposal: enforcing unit testing in gcl
We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see what
it misses and where its false positives are too annoying.
What changes might need tests?
- Any new source file (.cc, .cpp, .m, or .mm), or
- Any new method added to a source or header (.h file
- A new method is identified by a flush-left non-comment line that has
( somewhere before the next flush-left line and either ends with
{ or has {
at the start of the next flush-left line.
What counts as a test?
- Any change to any code file whose name ends in test.* or tests.*
- This is very rough, but at least it shows that the contributor
thought about testing when making the patch
What do we do if we don't find a test?
- On 'gcl change', report a warning to the user
- On 'gcl upload', add a warning to the change description so the
reviewer sees it too
- Add an option to override this
Future possibilities
- Is it worth restricting the check to only public or protected methods?
- Since any real change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests (including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep layout
tests in the same repository
- Rather than adding a warning to the change description, it'd be nice to
have a separate warning in the review app, so it showed up no matter what
and the path author didn't have to override anything. But since we want the
warning in client-side 'gcl change' anyway, for now we'll keep it simple and
trust people.
Comments and volunteers welcome.
- Pam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Proposal: enforcing unit testing in gcl
I think this sounds like an excellent start. We can tweak it if we
notice things not working properly.
Brett
On Mon, Jan 5, 2009 at 3:25 PM, Pam Greene p...@chromium.org wrote:
We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see what
it misses and where its false positives are too annoying.
What changes might need tests?
Any new source file (.cc, .cpp, .m, or .mm), or
Any new method added to a source or header (.h file
A new method is identified by a flush-left non-comment line that has (
somewhere before the next flush-left line and either ends with { or has { at
the start of the next flush-left line.
What counts as a test?
Any change to any code file whose name ends in test.* or tests.*
This is very rough, but at least it shows that the contributor thought about
testing when making the patch
What do we do if we don't find a test?
On 'gcl change', report a warning to the user
On 'gcl upload', add a warning to the change description so the reviewer
sees it too
Add an option to override this
Future possibilities
Is it worth restricting the check to only public or protected methods?
Since any real change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests (including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep layout
tests in the same repository
Rather than adding a warning to the change description, it'd be nice to have
a separate warning in the review app, so it showed up no matter what and the
path author didn't have to override anything. But since we want the warning
in client-side 'gcl change' anyway, for now we'll keep it simple and trust
people.
Comments and volunteers welcome.
- Pam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Proposal: enforcing unit testing in gcl
Sounds great, Pam!
I think the gcl command line option to submit without a test should be:
gcl upload --i-am-a-test-slacker
Mike
On Mon, Jan 5, 2009 at 3:25 PM, Pam Greene p...@chromium.org wrote:
We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see what
it misses and where its false positives are too annoying.
What changes might need tests?
Any new source file (.cc, .cpp, .m, or .mm), or
Any new method added to a source or header (.h file
A new method is identified by a flush-left non-comment line that has (
somewhere before the next flush-left line and either ends with { or has { at
the start of the next flush-left line.
What counts as a test?
Any change to any code file whose name ends in test.* or tests.*
This is very rough, but at least it shows that the contributor thought about
testing when making the patch
What do we do if we don't find a test?
On 'gcl change', report a warning to the user
On 'gcl upload', add a warning to the change description so the reviewer
sees it too
Add an option to override this
Future possibilities
Is it worth restricting the check to only public or protected methods?
Since any real change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests (including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep layout
tests in the same repository
Rather than adding a warning to the change description, it'd be nice to have
a separate warning in the review app, so it showed up no matter what and the
path author didn't have to override anything. But since we want the warning
in client-side 'gcl change' anyway, for now we'll keep it simple and trust
people.
Comments and volunteers welcome.
- Pam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Proposal: enforcing unit testing in gcl
I think you also need a way to override this in some instances, since
there are methods (especially in views/ and the UI) for which there is
no simple way to verify correctness of an implementation in code -
e.g. a function that paints a series of bitmaps at specific locations.
(Nor would we really want to invest in one, since visual designs
evolve continuously over time).
-Ben
On Mon, Jan 5, 2009 at 3:25 PM, Pam Greene p...@chromium.org wrote:
We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see what
it misses and where its false positives are too annoying.
What changes might need tests?
Any new source file (.cc, .cpp, .m, or .mm), or
Any new method added to a source or header (.h file
A new method is identified by a flush-left non-comment line that has (
somewhere before the next flush-left line and either ends with { or has { at
the start of the next flush-left line.
What counts as a test?
Any change to any code file whose name ends in test.* or tests.*
This is very rough, but at least it shows that the contributor thought about
testing when making the patch
What do we do if we don't find a test?
On 'gcl change', report a warning to the user
On 'gcl upload', add a warning to the change description so the reviewer
sees it too
Add an option to override this
Future possibilities
Is it worth restricting the check to only public or protected methods?
Since any real change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests (including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep layout
tests in the same repository
Rather than adding a warning to the change description, it'd be nice to have
a separate warning in the review app, so it showed up no matter what and the
path author didn't have to override anything. But since we want the warning
in client-side 'gcl change' anyway, for now we'll keep it simple and trust
people.
Comments and volunteers welcome.
- Pam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Proposal: enforcing unit testing in gcl
Yes, there's an override option in the proposal below. It'll still give a
warning on 'gcl change', but that's harmless.
- Pam
On Mon, Jan 5, 2009 at 4:08 PM, Ben Goodger (Google) b...@chromium.orgwrote:
I think you also need a way to override this in some instances, since
there are methods (especially in views/ and the UI) for which there is
no simple way to verify correctness of an implementation in code -
e.g. a function that paints a series of bitmaps at specific locations.
(Nor would we really want to invest in one, since visual designs
evolve continuously over time).
-Ben
On Mon, Jan 5, 2009 at 3:25 PM, Pam Greene p...@chromium.org wrote:
We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a
policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's
a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see
what
it misses and where its false positives are too annoying.
What changes might need tests?
Any new source file (.cc, .cpp, .m, or .mm), or
Any new method added to a source or header (.h file
A new method is identified by a flush-left non-comment line that has (
somewhere before the next flush-left line and either ends with { or has {
at
the start of the next flush-left line.
What counts as a test?
Any change to any code file whose name ends in test.* or tests.*
This is very rough, but at least it shows that the contributor thought
about
testing when making the patch
What do we do if we don't find a test?
On 'gcl change', report a warning to the user
On 'gcl upload', add a warning to the change description so the reviewer
sees it too
Add an option to override this
Future possibilities
Is it worth restricting the check to only public or protected methods?
Since any real change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests
(including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep
layout
tests in the same repository
Rather than adding a warning to the change description, it'd be nice to
have
a separate warning in the review app, so it showed up no matter what and
the
path author didn't have to override anything. But since we want the
warning
in client-side 'gcl change' anyway, for now we'll keep it simple and
trust
people.
Comments and volunteers welcome.
- Pam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Has your computer melted?
I just checked in a change to use /MP for all compiles, which is a secret undocumented flag that does parallel compiles within each project. Please let me know of your computer melts or becomes unusable during a compile. It should more efficiently use all of your CPUs when doing regular Visual Studio builds (it will have no effect on IncrediBuilds). You can remove it from essential.vsprops if it's causing problems. We tested on Carlos' 2-processor system and it pegged the CPU more, although it's not clear if it's a lot faster than before. On my 4-processor system, a build of just chrome_exe and all of it's dependencies went from 25 to 16 minutes after using this flag. So if you hate IncrediBuild, life might actually be tolerable without it for fast systems. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Has your computer melted?
Awesome Brett! My Quad Core processor is using all the 4 CPU :) 94% constant :x Mine took 20minutes to build from scratch, big improvement! I was doing many other stuff at the same time, Java (Eclipse), C# Visual Studio, browsing. Awesome Improvement, thanks! No need to wait 40 minutes :) On Mon, Jan 5, 2009 at 7:32 PM, Brett Wilson bre...@chromium.org wrote: I just checked in a change to use /MP for all compiles, which is a secret undocumented flag that does parallel compiles within each project. Please let me know of your computer melts or becomes unusable during a compile. It should more efficiently use all of your CPUs when doing regular Visual Studio builds (it will have no effect on IncrediBuilds). You can remove it from essential.vsprops if it's causing problems. We tested on Carlos' 2-processor system and it pegged the CPU more, although it's not clear if it's a lot faster than before. On my 4-processor system, a build of just chrome_exe and all of it's dependencies went from 25 to 16 minutes after using this flag. So if you hate IncrediBuild, life might actually be tolerable without it for fast systems. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] What WebKit goodies do we get in 155?
I'm working on release notes for 155. The big addition in 155 (vs the 154 code we've been releasing) is the WebKit update. What have we gained since we started merging to WebKit's trunk? So far, I've got -- full page zoom -- autoscroll -- CSS gradients -- CSS canvas drawing -- (partly) CSS masks -- (partly?) CSS reflections Can you think of other features worth noting in release notes? Thanks, Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: What WebKit goodies do we get in 155?
How about mouse gestures, middle scroll button On Tue, Jan 6, 2009 at 12:30 AM, Mark Larson (Google) m...@chromium.orgwrote: I'm working on release notes for 155. The big addition in 155 (vs the 154 code we've been releasing) is the WebKit update. What have we gained since we started merging to WebKit's trunk? So far, I've got -- full page zoom -- autoscroll -- CSS gradients -- CSS canvas drawing -- (partly) CSS masks -- (partly?) CSS reflections Can you think of other features worth noting in release notes? Thanks, Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: What WebKit goodies do we get in 155?
Mark I shouldn't have added that extra comma, I just meant autoscroll. Sorry for the confusion :) On Tue, Jan 6, 2009 at 1:24 AM, Mark Larson (Google) m...@chromium.orgwrote: On Mon, Jan 5, 2009 at 21:57, Mohamed Mansour m0.interact...@gmail.comwrote: How about mouse gestures, middle scroll button Middle scroll button is what I mean by 'autoscroll'. What mouse gestures have been added? On Tue, Jan 6, 2009 at 12:30 AM, Mark Larson (Google) m...@chromium.orgwrote: I'm working on release notes for 155. The big addition in 155 (vs the 154 code we've been releasing) is the WebKit update. What have we gained since we started merging to WebKit's trunk? So far, I've got -- full page zoom -- autoscroll -- CSS gradients -- CSS canvas drawing -- (partly) CSS masks -- (partly?) CSS reflections Can you think of other features worth noting in release notes? Thanks, Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: What WebKit goodies do we get in 155?
I remember now, Mouse Gestures such as Forward/Backward mouse button, to navigate web pages.Its not on 154 but on 155 On Tue, Jan 6, 2009 at 1:30 AM, Mohamed Mansour m0.interact...@gmail.comwrote: Mark I shouldn't have added that extra comma, I just meant autoscroll. Sorry for the confusion :) On Tue, Jan 6, 2009 at 1:24 AM, Mark Larson (Google) m...@chromium.orgwrote: On Mon, Jan 5, 2009 at 21:57, Mohamed Mansour m0.interact...@gmail.comwrote: How about mouse gestures, middle scroll button Middle scroll button is what I mean by 'autoscroll'. What mouse gestures have been added? On Tue, Jan 6, 2009 at 12:30 AM, Mark Larson (Google) m...@chromium.orgwrote: I'm working on release notes for 155. The big addition in 155 (vs the 154 code we've been releasing) is the WebKit update. What have we gained since we started merging to WebKit's trunk? So far, I've got -- full page zoom -- autoscroll -- CSS gradients -- CSS canvas drawing -- (partly) CSS masks -- (partly?) CSS reflections Can you think of other features worth noting in release notes? Thanks, Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Stabilization Effort Daily Report
*Report for 2009-1-5* The Layout Tests class went well. We are going to record it next week so people can watch it on demand. Eric's slides are at http://www.corp.google.com/~ericroman/layout/http://www.corp.google.com/%7Eericroman/layout/ I appreciate the enthusiasm! It is a good idea to look at http://code.google.com/p/chromium/wiki/StabilizeTrunk for the latest information. The Wiki also includes information on how to help fix layout tests. This is a great way to get involved and begin to learn the codebase. If you have ever wanted to be an open source developer then this is a great way to start! *Layout Tests* Each day the report at http://www.corp.google.com/~jonc/layout-summary.htmlhttp://www.corp.google.com/%7Ejonc/layout-summary.html will be updated with recent results. This report allows you to find layout tests that are failing on all platforms. It also makes it clear which directories have the most failures if you would like to work in a specific area. As always be sure to sign up at http://spreadsheets.google.com/ccc?key=pMwul3Seofg448Q1VFJjsJAhl=en if you are going to work on a layout test. We don't want to step on each other's toes. Our hopes that the *jultomte http://en.wikipedia.org/wiki/Tomte * would fix all the layout tests did not pan out so we are going to have to do it ourselves. Here is where we currently stand: [image: To+Be+Fixed=1.4][image: All+Tests=74.9][image: Want+To+Pass=90.4] The Fixed percentage is based on the layout tests in the tests_fixablehttp://src.chromium.org/viewvc/chrome/trunk/src/webkit/tools/layout_tests/test_lists/tests_fixable.txt?view=markup file. These are tests we are currently ignoring because we know they fail. As soon as we fix them we move them out of fixable so this number does not tend to get very high. It can momentarily spike between the time we fix the test and update the file. All tests is based on all available layout tests including those that we are currently not trying to pass. There are tests in this group which are known to be bad or relate to future technologies. Want to Pass is based on the tests that we need to be passing before we will ship a revision of the browser. Getting this number as high as possible is the goal of the stabilization effort. Some of these tests are failing due to subtle changes that require the test to be re-baselined. *Purify Bugs (Memory)* We have resolved 10 of the 40 Purify issues. That is two more than before the holiday break. *Regressions* We have resolved 11 of 25 regressions. That is one more since before the holiday break. *Other bugs* We have also resolved 12 of the 43 other bugs. That is 2 more than before the break but there are also 2 new ones. So our bug burndown chart looks like this: As long as we keep the red line below the blue line we are on track for the bugs. Keep in mind that this does not include the work on Layout Tests. You will find a lot more information about the Stabilization effort on the Wiki at http://code.google.com/p/chromium/wiki/StabilizeTrunk --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: What WebKit goodies do we get in 155?
If 155 is coming off of trunk, it has postMessage(), which is exciting for us security wonks. Adam On Mon, Jan 5, 2009 at 9:30 PM, Mark Larson (Google) m...@chromium.org wrote: I'm working on release notes for 155. The big addition in 155 (vs the 154 code we've been releasing) is the WebKit update. What have we gained since we started merging to WebKit's trunk? So far, I've got -- full page zoom -- autoscroll -- CSS gradients -- CSS canvas drawing -- (partly) CSS masks -- (partly?) CSS reflections Can you think of other features worth noting in release notes? Thanks, Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
