VS-Addin Build/Test/Install scripts

Adding scripts to build and test the add-in, as well as installer script.

Only the .bat and .py files need reviewing.  Other changes were just from moving files around.

Cleaned up python scripts.

Will now parse the output.

Fixed test script to delete the results file to allow rerunning tests.

Packaged zip file now created

Added build and test scripts

BUG=136414
TEST=

Committed: https://code.google.com/p/nativeclient-sdk/source/detail?r=1402

[binji, 2012-07-17 00:05:58]

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/InstallerResources/install.py (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:17: if
platform.system() != 'Windows':
Use the standard python file structure:
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=Main#Main

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:18: raise
Exception('Must install to Windows system')
2 space indent

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:21:
nacl_sdk_root = os.path.expandvars('%NACL_SDK_ROOT%')
just use os.getenv('NACL_SDK_ROOT', None) for this and check against None below.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:22: chrome_path
= os.path.expandvars('%CHROME_PATH%')
same here

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/build.bat (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/build.bat:2: @call
"%VS100COMNTOOLS%vsvars32.bat"
Where is this script being run? Can we assume the user has launched the Visual
Studio Command prompt already?

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/check_test_results.py (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:15: 
Use main function here as well.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:16: if (len(sys.argv) <
2):
remove parens around if

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:17: print 'Must provide
path to the Results.trx file'
indent 2 spaces, here and elsewhere

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:33: if
(result.attrib['outcome'] != 'Passed'):
remove parens around if

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:35: print
result.attrib['testName']
What does this output look like? It seems like it would be preferable to output
all attributes on one line.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/create_package.py (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/create_package.py:22: assembly_directory =
"../../out/NativeClientVSAddIn/Debug"
factor output directory into its own variable and use os.path.join for these
directories.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/create_package.py:36:
shutil.copy(os.path.join(resource_directory, 'NativeClientVSAddIn.AddIn'),
make list of files to copy and loop over it.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/create_package.py:45: for file_name in
glob.glob(zip_directory + "/*"):
rather than glob, just use the copy list from above.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/test.bat (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/test.bat:11: @IF EXIST
%out_directory%\%mstest_file% del %out_directory%\%mstest_file%
nit: does if exist have to be caps? Nothing else is...

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/test.bat:17: @python check_test_results.py
%out_directory%\%mstest_file%
don't you need to set the errorlevel for this batch file based on the returncode
from the python script...?

[tysand, 2012-07-17 01:18:18]

Fixed Ben's suggestions.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/InstallerResources/install.py (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:17: if
platform.system() != 'Windows':
On 2012/07/17 00:05:58, binji wrote:
> Use the standard python file structure:
>
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=Main#Main

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:18: raise
Exception('Must install to Windows system')
On 2012/07/17 00:05:58, binji wrote:
> 2 space indent

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:21:
nacl_sdk_root = os.path.expandvars('%NACL_SDK_ROOT%')
On 2012/07/17 00:05:58, binji wrote:
> just use os.getenv('NACL_SDK_ROOT', None) for this and check against None
below.

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:22: chrome_path
= os.path.expandvars('%CHROME_PATH%')
On 2012/07/17 00:05:58, binji wrote:
> same here

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/build.bat (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/build.bat:2: @call
"%VS100COMNTOOLS%vsvars32.bat"
Why assume that? This way the script can be run from any regular command prompt.
Even if they are already running from a VS command prompt I think the
vsvars32.bat script won't hurt anything, it just sets environment variables.
On 2012/07/17 00:05:58, binji wrote:
> Where is this script being run? Can we assume the user has launched the Visual
> Studio Command prompt already?

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/check_test_results.py (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:15: 
On 2012/07/17 00:05:58, binji wrote:
> Use main function here as well.

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:16: if (len(sys.argv) <
2):
On 2012/07/17 00:05:58, binji wrote:
> remove parens around if

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:17: print 'Must provide
path to the Results.trx file'
On 2012/07/17 00:05:58, binji wrote:
> indent 2 spaces, here and elsewhere

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:33: if
(result.attrib['outcome'] != 'Passed'):
On 2012/07/17 00:05:58, binji wrote:
> remove parens around if

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/check_test_results.py:35: print
result.attrib['testName']
On 2012/07/17 00:05:58, binji wrote:
> What does this output look like? It seems like it would be preferable to
output
> all attributes on one line.

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/create_package.py (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/create_package.py:22: assembly_directory =
"../../out/NativeClientVSAddIn/Debug"
On 2012/07/17 00:05:58, binji wrote:
> factor output directory into its own variable and use os.path.join for these
> directories.

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/create_package.py:36:
shutil.copy(os.path.join(resource_directory, 'NativeClientVSAddIn.AddIn'),
On 2012/07/17 00:05:58, binji wrote:
> make list of files to copy and loop over it.

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/create_package.py:45: for file_name in
glob.glob(zip_directory + "/*"):
On 2012/07/17 00:05:58, binji wrote:
> rather than glob, just use the copy list from above.

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
File visual_studio/NativeClientVSAddIn/test.bat (right):

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/test.bat:11: @IF EXIST
%out_directory%\%mstest_file% del %out_directory%\%mstest_file%
On 2012/07/17 00:05:58, binji wrote:
> nit: does if exist have to be caps? Nothing else is...

Done.

http://codereview.chromium.org/10790023/diff/1/visual_studio/NativeClientVSAd...
visual_studio/NativeClientVSAddIn/test.bat:17: @python check_test_results.py
%out_directory%\%mstest_file%
On 2012/07/17 00:05:58, binji wrote:
> don't you need to set the errorlevel for this batch file based on the
returncode
> from the python script...?

Done.

[noelallen1, 2012-07-17 01:34:59]

LGTM

http://codereview.chromium.org/10790023/diff/11/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/check_test_results.py (right):

http://codereview.chromium.org/10790023/diff/11/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/check_test_results.py:44: main()
sys.exit(main())
So that you return the result to the caller

[tysand, 2012-07-17 01:44:41]

http://codereview.chromium.org/10790023/diff/11/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/check_test_results.py (right):

http://codereview.chromium.org/10790023/diff/11/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/check_test_results.py:44: main()
On 2012/07/17 01:34:59, noelallen1 wrote:
> sys.exit(main())
> So that you return the result to the caller

Done.

[binji, 2012-07-17 17:38:55]

lgtm

FYI, it is customary to wait for an lgtm from a reviewer if he gives feedback...
it's no big deal here, but sometimes a reviewer won't give all feedback in one
response. I also like to be able to see that you were following the feedback the
way I expected.

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/InstallerResources/install.py (right):

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:22:
nacl_sdk_root = os.path.expandvars('%NACL_SDK_ROOT%', None)
I still like os.getenv better here (it doesn't require the platform specific
%VAR%), but OK

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:24: if
nacl_sdk_root == None:
Tests against None are usually written

if nacl_sdk_root is None:

or

if nacl_sdk_root:

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/install.py:30: addInDir =
os.path.expandvars(
nit: should be add_in_dir

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/create_package.py (right):

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/create_package.py:13: import glob
nit: can remove, not used anymore

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/create_package.py:15: import shutil
same here

http://codereview.chromium.org/10790023/diff/12/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/create_package.py:31: FILE_LIST = [
Nice, a lot easier to read now