Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
12248009:
Created unit test for PrinterJobHandler (Closed)
|
Created:
7 years, 10 months ago by Noam Samuel (WRONG ACCOUNT) Modified:
7 years, 10 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@test_printerjobhandler Visibility:
Public. |
DescriptionCreated unit test for PrinterJobHandler
Created file printer_job_handler_unittest.cc containing unit test for PrinterJobHandler
and fixtures that will allow the building of more sophisticated unit tests in the future.
Currently, the test only tests end-to-end printing of a single job with no errors, but
facilities exist in the unit test code to write more sophisticated tests in the future.
BUG=176361
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=183376
Patch Set 1 : Added changes made to the build files #Patch Set 2 : Fixing problems with previous patch set #Patch Set 3 : Adding namespace close comments and newline before EOF #
Total comments: 16
Patch Set 4 : #
Total comments: 22
Patch Set 5 : #
Total comments: 12
Patch Set 6 : #Patch Set 7 : #
Total comments: 8
Patch Set 8 : #Patch Set 9 : Updated to latest version of FakeURLFetcher and Fixed for CLANG compilation #
Total comments: 6
Patch Set 10 : #Patch Set 11 : #Patch Set 12 : #Patch Set 13 : #Patch Set 14 : #
Messages
Total messages: 28 (0 generated)
Whoops, looks like this patch set needs more work on fixing lint errors (and a one patching issue) before being reviewed. My bad. I will re-send it when it's ready.
Whoops, looks like this patch set needs more work on fixing lint errors (and a one patching issue) before being reviewed. My bad. I will re-send it when it's ready.
OK, I fixed the lint issues with the code. This delta is ready to be reviewed. Two lint warnings remain, but as far as I can tell, they stem from a bug in the linter causing it to misidentify Google Mock method signatures as typecasts.
https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.gypi File chrome/chrome_tests_unit.gypi (right): https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.g... chrome/chrome_tests_unit.gypi:1605: 'service/cloud_print/printer_job_handler_unittest_constants.h', Why do you need separate constants file? https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest_constants.h (right): https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest_constants.h:15: extern const char kExamplePrintTicket[]; empty line between constants are unnecessary https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest_constants.h:52: } // namespace cloud_print empty line before line 52
https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.gypi File chrome/chrome_tests_unit.gypi (right): https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.g... chrome/chrome_tests_unit.gypi:1605: 'service/cloud_print/printer_job_handler_unittest_constants.h', On 2013/02/12 21:45:15, Vitaly Buka wrote: > Why do you need separate constants file? The constants were becoming very long and cumbersome by their own right, so I factored them out into a separate file. https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest_constants.h (right): https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest_constants.h:15: extern const char kExamplePrintTicket[]; On 2013/02/12 21:45:15, Vitaly Buka wrote: > empty line between constants are unnecessary Done. https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest_constants.h:52: } // namespace cloud_print On 2013/02/12 21:45:15, Vitaly Buka wrote: > empty line before line 52 Done.
https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:211: scoped_refptr<TestURLFetcherCallback > url_callback_; no need space before > https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:254: &TestURLFetcherCallback::CreateURLFetcher, You can avoid base::RefCounted for TestURLFetcherCallback by base::Bind(..., base::Unretained(&url_callback_) https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:259: recomended places for gtest initialization and shutdown are virtual void SetUp() virtual void TearDown() https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:436: } empty line before } // namespace cloud_print
https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.gypi File chrome/chrome_tests_unit.gypi (right): https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.g... chrome/chrome_tests_unit.gypi:1605: 'service/cloud_print/printer_job_handler_unittest_constants.h', usually we don't do that for unit tests. So please move them there. On 2013/02/12 21:53:18, Noam Samuel wrote: > On 2013/02/12 21:45:15, Vitaly Buka wrote: > > Why do you need separate constants file? > > The constants were becoming very long and cumbersome by their own right, so I > factored them out into a separate file.
Do you really need consts in a separate file? Do you plan using them is some other unittests as well? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:42: net::URLRequestContextGetter* GetRequestContextGetter() { virtual https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:52: CloudPrintURLFetcher* CreateCloudPrintURLFetcher() { virtual https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:65: net::FakeURLFetcher* CreateURLFetcher( can you move url to this line and align the rest of the parameters? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:68: const std::string& response_data, bool success) { split this to two lines https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:89: protected: protected or private here? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:97: public: public: first, then protected: ? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:109: protected: protected or private here? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:117: public: public: first, then protected: ? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:129: protected: protected or private here? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:131: public: public: first, then protected: ? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:148: protected: protected or private here? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:153: public: public: first, then protected: ? https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:210: MessageLoopForIO loop_; Functions first, variables second https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:254: &TestURLFetcherCallback::CreateURLFetcher, move this to Bind( line
https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.gypi File chrome/chrome_tests_unit.gypi (right): https://codereview.chromium.org/12248009/diff/3011/chrome/chrome_tests_unit.g... chrome/chrome_tests_unit.gypi:1605: 'service/cloud_print/printer_job_handler_unittest_constants.h', On 2013/02/12 22:00:22, Vitaly Buka wrote: > usually we don't do that for unit tests. > So please move them there. > > On 2013/02/12 21:53:18, Noam Samuel wrote: > > On 2013/02/12 21:45:15, Vitaly Buka wrote: > > > Why do you need separate constants file? > > > > The constants were becoming very long and cumbersome by their own right, so I > > factored them out into a separate file. > Done. https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:211: scoped_refptr<TestURLFetcherCallback > url_callback_; On 2013/02/12 21:57:08, Vitaly Buka wrote: > no need space before > Done. https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:254: &TestURLFetcherCallback::CreateURLFetcher, On 2013/02/12 21:57:08, Vitaly Buka wrote: > You can avoid base::RefCounted for TestURLFetcherCallback by > base::Bind(..., base::Unretained(&url_callback_) Done. https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:259: On 2013/02/12 21:57:08, Vitaly Buka wrote: > recomended places for gtest initialization and shutdown are > virtual void SetUp() > virtual void TearDown() Done. https://codereview.chromium.org/12248009/diff/3011/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:436: } On 2013/02/12 21:57:08, Vitaly Buka wrote: > empty line before } // namespace cloud_print Done. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:42: net::URLRequestContextGetter* GetRequestContextGetter() { On 2013/02/12 22:06:18, gene wrote: > virtual Done. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:52: CloudPrintURLFetcher* CreateCloudPrintURLFetcher() { On 2013/02/12 22:06:18, gene wrote: > virtual Done. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:65: net::FakeURLFetcher* CreateURLFetcher( On 2013/02/12 22:06:18, gene wrote: > can you move url to this line and align the rest of the parameters? Done. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:68: const std::string& response_data, bool success) { On 2013/02/12 22:06:18, gene wrote: > split this to two lines Done. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:89: protected: On 2013/02/12 22:06:18, gene wrote: > protected or private here? Fixed to private for all such cases. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:97: public: On 2013/02/12 22:06:18, gene wrote: > public: first, then protected: ? Fixed for all such cases https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:210: MessageLoopForIO loop_; On 2013/02/12 22:06:18, gene wrote: > Functions first, variables second Done. https://codereview.chromium.org/12248009/diff/6007/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:254: &TestURLFetcherCallback::CreateURLFetcher, On 2013/02/12 22:06:18, gene wrote: > move this to Bind( line Done.
https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:203: : public CloudPrintURLFetcherFactory { http://www.corp.google.com/eng/doc/cppguide.xml?showone=Interfaces#Interfaces Please fix CloudPrintURLFetcherFactory, it must have virtual destructor Also http://www.corp.google.com/eng/doc/cppguide.xml?showone=Inheritance#Inheritance Make your destructor virtual if necessary. If your class has virtual methods, its destructor should be virtual. https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:245: Usually I add virtual destructors even if just base class has virtual method and destructor like here. https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:254: bool SetDelegate(PrintSystem::PrintServerWatcher::Delegate* d) { This should be void set_delegate()? http://www.corp.google.com/eng/doc/cppguide.xml?showone=Function_Names#Functi... https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:274: delegate_ = d; Same https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:292: PrintSystem::JobSpooler::Delegate* Delegate() { return delegate_; } this should be delegate() const https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:360: void TearDown(); Use OVERRIDE for all virtual overrides virtual void SetUp() OVERRIDE virtual void TearDown() OVERRIDE
https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:203: : public CloudPrintURLFetcherFactory { 1. Will submit interface fix as separate CL 2. Added virtual destructor to class On 2013/02/12 23:53:26, Vitaly Buka wrote: > http://www.corp.google.com/eng/doc/cppguide.xml?showone=Interfaces#Interfaces > > Please fix CloudPrintURLFetcherFactory, it must have virtual destructor > > Also > http://www.corp.google.com/eng/doc/cppguide.xml?showone=Inheritance#Inheritance > Make your destructor virtual if necessary. If your class has virtual methods, > its destructor should be virtual. > https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:245: On 2013/02/12 23:53:26, Vitaly Buka wrote: > Usually I add virtual destructors even if just base class has virtual method and > destructor like here. Done. https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:254: bool SetDelegate(PrintSystem::PrintServerWatcher::Delegate* d) { Turns out this setter was unnecessary. Removed it. On 2013/02/12 23:53:26, Vitaly Buka wrote: > This should be void set_delegate()? > http://www.corp.google.com/eng/doc/cppguide.xml?showone=Function_Names#Functi... https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:274: delegate_ = d; Ditto. On 2013/02/12 23:53:26, Vitaly Buka wrote: > Same https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:292: PrintSystem::JobSpooler::Delegate* Delegate() { return delegate_; } On 2013/02/12 23:53:26, Vitaly Buka wrote: > this should be delegate() const Done. https://codereview.chromium.org/12248009/diff/4006/chrome/service/cloud_print... chrome/service/cloud_print/printer_job_handler_unittest.cc:360: void TearDown(); On 2013/02/12 23:53:26, Vitaly Buka wrote: > Use OVERRIDE for all virtual overrides > virtual void SetUp() OVERRIDE > virtual void TearDown() OVERRIDE Done.
lgtm gene, do you have anything to add?
lgtm Could you please add empty virtual destructor to CloudPrintURLFetcherFactory? I was wrong asking you to remove it. + a few nits below https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:255: virtual ~MockPrintServerWatcher() {} Could you please move destructor next to contructor https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:271: virtual ~MockPrinterWatcher() {} Could you please move destructor next to contructor https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:292: virtual ~MockJobSpooler() {} Could you please move destructor next to contructor https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:347: virtual ~MockPrintSystem() {} Could you please move destructor next to contructor
I submitted the virtual destructor for CloudPrintURLFetcherFactory as a separate CL at https://codereview.chromium.org/12230020/ https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:255: virtual ~MockPrintServerWatcher() {} On 2013/02/13 03:43:59, gene wrote: > Could you please move destructor next to contructor Done. https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:271: virtual ~MockPrinterWatcher() {} On 2013/02/13 03:43:59, gene wrote: > Could you please move destructor next to contructor Done. https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:292: virtual ~MockJobSpooler() {} On 2013/02/13 03:43:59, gene wrote: > Could you please move destructor next to contructor Done. https://codereview.chromium.org/12248009/diff/10009/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:347: virtual ~MockPrintSystem() {} On 2013/02/13 03:43:59, gene wrote: > Could you please move destructor next to contructor Done.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@google.com/12248009/17001
Hey, sorry to re-open this review, but two issues arose with the CL as it currently stands: 1. I had to adjust the code to match the changed interface in CL https://codereview.chromium.org/12211076/ 2. CLANG was having trouble compiling this unit test because it requires that ref counted classes have explicit private/protected destructors, so I added private explicit destructors for all the ref-counted test classes. For the mocks, this necessistates listing scoped_refptr<*> as a friend class as well as NiceMock<*> and scoped_refptr<NiceMock<*> >. I threw in similar friendship for StrictMock in case someone wants to use strict mocks of these in the future.
https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:227: net::FakeURLFetcher* fetcher = new net::FakeURLFetcher(url, d, put new() result into smart pointers if you can Like scoped_ptr<net::FakeURLFetcher> fetcher(new net::FakeURLFetcher()) Here is simple code, but in large function you can leak memory if you return netweet new and scoped_ptr https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:458: } Missaligned base::Bind(&TestURLFetcherCallback::CreateURLFetcher, base::Unretained(&url_callback_))) { https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:476: const GURL& url, http://dev.chromium.org/developers/coding-style It should be like OK2 example
https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... File chrome/service/cloud_print/printer_job_handler_unittest.cc (right): https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:227: net::FakeURLFetcher* fetcher = new net::FakeURLFetcher(url, d, On 2013/02/14 01:25:27, Vitaly Buka wrote: > put new() result into smart pointers if you can > Like > scoped_ptr<net::FakeURLFetcher> fetcher(new net::FakeURLFetcher()) > > Here is simple code, but in large function you can leak memory if you return > netweet new and scoped_ptr Done. https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:458: } On 2013/02/14 01:25:27, Vitaly Buka wrote: > Missaligned > base::Bind(&TestURLFetcherCallback::CreateURLFetcher, > base::Unretained(&url_callback_))) { Done. https://codereview.chromium.org/12248009/diff/22002/chrome/service/cloud_prin... chrome/service/cloud_print/printer_job_handler_unittest.cc:476: const GURL& url, On 2013/02/14 01:25:27, Vitaly Buka wrote: > http://dev.chromium.org/developers/coding-style > It should be like OK2 example Done.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@google.com/12248009/27001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@google.com/12248009/29005
On 2013/02/19 18:14:38, I haz the power (commit-bot) wrote: > CQ is trying da patch. Follow status at > https://chromium-status.appspot.com/cq/noamsml%40google.com/12248009/29005 FYI: Changed FilePath to base::FilePath to account for https://codereview.chromium.org/12282019/
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@google.com/12248009/46003
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@google.com/12248009/42003
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@google.com/12248009/42003
Message was sent while issue was closed.
Change committed as 183376 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
