chrome/installer/automation_test/chrome_installed.prop - Issue 20578004: Initial commit for the Automated Installer Testing Framework.

Side by Side Diff: chrome/installer/automation_test/chrome_installed.prop

Issue 20578004: Initial commit for the Automated Installer Testing Framework. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master

Patch Set: Created 7 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 {
	robertshield 2013/07/26 19:32:23 Suggest adding these files under a sub directory. Suggest adding these files under a sub directory. automation_test/data or something similar gab 2013/07/26 20:39:48 Right, I suggest a "config" directory instead of " Show quoted text On 2013/07/26 19:32:23, robertshield wrote: > Suggest adding these files under a sub directory. automation_test/data or > something similar Right, I suggest a "config" directory instead of "data" though. I also think this all should go in chrome/installer/mini_installer/test instead of automation_test (all tests are automated ;)!) grt (UTC plus 2) 2013/07/30 03:31:38 Judging by the current directory structure under s Show quoted text On 2013/07/26 20:39:48, gab wrote: > On 2013/07/26 19:32:23, robertshield wrote: > > Suggest adding these files under a sub directory. automation_test/data or > > something similar > > Right, I suggest a "config" directory instead of "data" though. > > I also think this all should go in chrome/installer/mini_installer/test instead > of automation_test (all tests are automated ;)!) Judging by the current directory structure under src/chrome, I think src/chrome/test/mini_installer is more appropriate than src/chrome/SOMETHING/test. Is there any overlap between the goals of this project and those of chrome/test/install_test? Can anything there be reused? grt (UTC plus 2) 2013/07/30 03:31:38 Consider giving these files a .json suffix so that Consider giving these files a .json suffix so that editors know the format. gab 2013/07/30 15:11:43 Makes sense, there already seems to be chrome/test Show quoted text On 2013/07/30 03:31:38, grt (no reviews Aug 3-11) wrote: > On 2013/07/26 20:39:48, gab wrote: > > On 2013/07/26 19:32:23, robertshield wrote: > > > Suggest adding these files under a sub directory. automation_test/data or > > > something similar > > > > Right, I suggest a "config" directory instead of "data" though. > > > > I also think this all should go in chrome/installer/mini_installer/test > instead > > of automation_test (all tests are automated ;)!) > > Judging by the current directory structure under src/chrome, I think > src/chrome/test/mini_installer is more appropriate than > src/chrome/SOMETHING/test. > > Is there any overlap between the goals of this project and those of > chrome/test/install_test? Can anything there be reused? Makes sense, there already seems to be chrome/test/mini_installer_test; those are the old mini_installer tests that are completely broken (and thus never used) and the new framework will completely replace them. We should probably just get rid of this first and then put the new framework's code there. WDYT? sukolsak 2013/07/30 15:40:32 I use .prop and .config suffixes in the spec to di Show quoted text On 2013/07/30 03:31:38, grt (no reviews Aug 3-11) wrote: > Consider giving these files a .json suffix so that editors know the format. I use .prop and .config suffixes in the spec to distinguish between property files and config files. Would it be confusing if all the data files have .json suffix? gab 2013/07/30 20:44:14 I prefer separate extensions to separate what we e Show quoted text On 2013/07/30 15:40:32, sukolsak wrote: > On 2013/07/30 03:31:38, grt (no reviews Aug 3-11) wrote: > > Consider giving these files a .json suffix so that editors know the format. > > I use .prop and .config suffixes in the spec to distinguish between property > files and config files. Would it be confusing if all the data files have .json > suffix? I prefer separate extensions to separate what we expect to find in those files. .json is too generic imo (i.e., the parser, after parsing the JSON expects specific things within the JSON, so it's more than just JSON) grt (UTC plus 2) 2013/07/31 17:20:05 I don't like the redundancy of chrome/test/mini_in Show quoted text On 2013/07/30 15:11:43, gab wrote: > On 2013/07/30 03:31:38, grt (no reviews Aug 3-11) wrote: > > On 2013/07/26 20:39:48, gab wrote: > > > On 2013/07/26 19:32:23, robertshield wrote: > > > > Suggest adding these files under a sub directory. automation_test/data or > > > > something similar > > > > > > Right, I suggest a "config" directory instead of "data" though. > > > > > > I also think this all should go in chrome/installer/mini_installer/test > > instead > > > of automation_test (all tests are automated ;)!) > > > > Judging by the current directory structure under src/chrome, I think > > src/chrome/test/mini_installer is more appropriate than > > src/chrome/SOMETHING/test. > > > > Is there any overlap between the goals of this project and those of > > chrome/test/install_test? Can anything there be reused? > > Makes sense, there already seems to be chrome/test/mini_installer_test; those > are the old mini_installer tests that are completely broken (and thus never > used) and the new framework will completely replace them. > > We should probably just get rid of this first and then put the new framework's > code there. > > WDYT? I don't like the redundancy of chrome/test/mini_installer_test. I think chrome/test/mini_installer is much better. grt (UTC plus 2) 2013/07/31 17:20:05 SGTM. Show quoted text On 2013/07/30 20:44:14, gab wrote: > On 2013/07/30 15:40:32, sukolsak wrote: > > On 2013/07/30 03:31:38, grt (no reviews Aug 3-11) wrote: > > > Consider giving these files a .json suffix so that editors know the format. > > > > I use .prop and .config suffixes in the spec to distinguish between property > > files and config files. Would it be confusing if all the data files have .json > > suffix? > > I prefer separate extensions to separate what we expect to find in those files. > > .json is too generic imo (i.e., the parser, after parsing the JSON expects > specific things within the JSON, so it's more than just JSON) SGTM.
	2 "RegistryEntries": {
	robertshield 2013/07/26 19:32:23 not sure, but it look like these might be tab char not sure, but it look like these might be tab characters? tabs are verboten in Chromium code and indents are usually two spaces. See http://www.chromium.org/developers/coding-style and specifically, http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Spaces... gab 2013/07/26 20:39:48 Presubmit scripts (also ran when uploading) should Show quoted text On 2013/07/26 19:32:23, robertshield wrote: > not sure, but it look like these might be tab characters? tabs are verboten in > Chromium code and indents are usually two spaces. > > See http://www.chromium.org/developers/coding-style and specifically, > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Spaces... Presubmit scripts (also ran when uploading) should catch tabs if any. I know because this happened to me when my editor's config broke earlier today!
	3 "HKEY_CURRENT_USER\\Software\\Google\\Update\\Clients\\{8A69D345 -D564-463c-AFF1-A69D9E530F96}": {"expected": true}
	gab 2013/07/26 20:39:48 Prefer to terminate all lists/dicts in JSON with c Prefer to terminate all lists/dicts in JSON with commas after the last item, this is the preferred style across chromium when the language allows it. The benefit is that you can add and remove items without worrying about fixing the comma of the last item... sukolsak 2013/07/26 21:31:03 The JSON spec does not allow trailing commas. Show quoted text On 2013/07/26 20:39:48, gab wrote: > Prefer to terminate all lists/dicts in JSON with commas after the last item, > this is the preferred style across chromium when the language allows it. > > The benefit is that you can add and remove items without worrying about fixing > the comma of the last item... The JSON spec does not allow trailing commas. gab 2013/07/29 20:40:26 Ah :(, does the python parser allow them anyways? Show quoted text On 2013/07/26 21:31:03, sukolsak wrote: > On 2013/07/26 20:39:48, gab wrote: > > Prefer to terminate all lists/dicts in JSON with commas after the last item, > > this is the preferred style across chromium when the language allows it. > > > > The benefit is that you can add and remove items without worrying about fixing > > the comma of the last item... > > The JSON spec does not allow trailing commas. Ah :(, does the python parser allow them anyways? I think most of our existing JSON-like formats use trailing commas. grt (UTC plus 2) 2013/07/30 03:31:38 Maybe you're thinking of our GYP files, which are Show quoted text On 2013/07/29 20:40:26, gab wrote: > I think most of our existing JSON-like formats use trailing commas. Maybe you're thinking of our GYP files, which are python rather than JSON. I believe that Chrome's JSON parser will tell you off if you try to slip in a trailing comma (been there, done that, wasted 20 minutes trying to figure out why my master_preferences were being ignored).
	4 }

	5 }

OLD	NEW

« no previous file with comments | « no previous file | chrome/installer/automation_test/chrome_not_installed.prop » ('j') | chrome/installer/automation_test/chromeframe_installed.prop » ('J')