Enable accessibility testing for the bookmark browser test.

chrome/test/base/in_process_browser_test.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_TEST_BASE_IN_PROCESS_BROWSER_TEST_H_
 #define CHROME_TEST_BASE_IN_PROCESS_BROWSER_TEST_H_
 
 #include "base/compiler_specific.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_base.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/page_transition_types.h"
 
 namespace base {
 
 class CommandLine;
 
 #if defined(OS_MACOSX)
@@ skipping 75 matching lines @@
   ~InProcessBrowserTest() override;
 
   // Configures everything for an in process browser test, then invokes
   // BrowserMain. BrowserMain ends up invoking RunTestOnMainThreadLoop.
   void SetUp() override;
 
   // Restores state configured in SetUp.
   void TearDown() override;
 
  protected:
+  // Constant for accessibility audit.
+  static const std::string kExpectedAccessibilityResults;

[Jay Civelli, 2015/02/05 23:29:09]

Does it need to be part of the class?

[hcarmona, 2015/02/06 22:50:53]

With the other change to make the parameter for the RunAccessibilityChecks into
an error message it's no longer necessary to have this constant at all.

+
   // Returns the browser created by CreateBrowser.
   Browser* browser() const { return browser_; }
 
   // Convenience methods for adding tabs to a Browser.
   void AddTabAtIndexToBrowser(Browser* browser,
                               int index,
                               const GURL& url,
                               ui::PageTransition transition,
                               bool check_navigation_success);
   void AddTabAtIndex(int index,
@@ skipping 24 matching lines @@
   Browser* CreateBrowserForPopup(Profile* profile);
 
   // Creates a browser for an application and waits for it to load and shows
   // the browser.
   Browser* CreateBrowserForApp(const std::string& app_name, Profile* profile);
 
   // Called from the various CreateBrowser methods to add a blank tab, wait for
   // the navigation to complete, and show the browser's window.
   void AddBlankTabAndShow(Browser* browser);
 
+  // Disables running of accessibility tests for a particular test case. This is
+  // only meaningful when called from IN_PROC_BROWSER_TEST_F to override the
+  // value of run_accessibility_checks_.
+  void DisableAccessibilityChecks();

[Jay Civelli, 2015/02/05 23:29:09]

How about having only 1 method?
SetAccessibilityCheckEnabled(bool enabled)

[hcarmona, 2015/02/06 22:50:53]

I think it makes sense. Collapsed both of these into 1 method.

+
+  // Enables running of accessibility tests for a particular test case. This is
+  // only meaningful when called from IN_PROC_BROWSER_TEST_F to override the
+  // value of run_accessibility_checks_.
+  void EnableAccessibilityChecks();
+
 #if !defined OS_MACOSX
   // Return a CommandLine object that is used to relaunch the browser_test
   // binary as a browser process. This function is deliberately not defined on
   // the Mac because re-using an existing browser process when launching from
   // the command line isn't a concept that we support on the Mac; AppleEvents
   // are the Mac solution for the same need. Any test based on these functions
   // doesn't apply to the Mac.
   base::CommandLine GetCommandLineForRelaunch();
 #endif
 
@@ skipping 10 matching lines @@
 
   void set_open_about_blank_on_browser_launch(bool value) {
     open_about_blank_on_browser_launch_ = value;
   }
 
   // This must be called before RunTestOnMainThreadLoop() to have any effect.
   void set_multi_desktop_test(bool multi_desktop_test) {
     multi_desktop_test_ = multi_desktop_test;
   }
 
+  // This should be called in a sub-class constructor to enable/disable
+  // accessibility testing for all test cases.
+  void set_run_accessibility_checks(bool run_accessibility_checks) {

[Jay Civelli, 2015/02/05 23:29:09]

I am a bit confused by this.
You can either enable it always (in which case RunAccessibilityChecks is called
by the test framework for you for all test cases) or on a test case bases by
calling RunAccessibilityChecks from your test.
Did I get it right?

[hcarmona, 2015/02/06 22:50:53]

I think you're right and this is too confusing. I've simplified this a bit by
only having one method now. This method can be called in SetUpOnMainThread to
enable a11y audit for all tests but can still be called from the test body to
enable/disable only one test if necessary.

A test shouldn't need to call RunAccessibilityChecks because they can just
enable it. It's here so that it can be tested in
InProcessAccessibilityBrowsertest.

[Jay Civelli, 2015/02/07 01:11:12]

This looks much better, thanks!

+    run_accessibility_checks_ = run_accessibility_checks;
+  }
+
+  // Will run accessibility checks and fail the test if there are any issues.
+  void RunAccessibilityChecks(std::string *test_result);

[Jay Civelli, 2015/02/05 23:29:09]

std::string*

[hcarmona, 2015/02/06 22:50:53]

Done.

+
  private:
   // Creates a user data directory for the test if one is needed. Returns true
   // if successful.
   virtual bool CreateUserDataDirectory() WARN_UNUSED_RESULT;
 
   // Quits all open browsers and waits until there are no more browsers.
   void QuitBrowsers();
 
   // Prepare command line that will be used to launch the child browser process
   // with an in-process test.
   void PrepareTestCommandLine(base::CommandLine* command_line);
 
+  // This method loads the javascript accessibility library.
+  // Returns false on loading error. Accessibility audit will fail if library is

[Jay Civelli, 2015/02/05 23:29:09]

Should it just assert if it fails to load?

[hcarmona, 2015/02/06 22:50:53]

Only called one place. Moved this method's contents into RunAccessibilityChecks.

+  // not loaded.
+  bool LoadAccessibilityLibrary(content::WebContents* web_contents)
+      WARN_UNUSED_RESULT;
+
   // Browser created from CreateBrowser.
   Browser* browser_;
 
   // Temporary user data directory. Used only when a user data directory is not
   // specified in the command line.
   base::ScopedTempDir temp_user_data_dir_;
 
   // True if we should exit the tests after the last browser instance closes.
   bool exit_when_last_browser_closes_;
 
   // True if the about:blank tab should be opened when the browser is launched.
   bool open_about_blank_on_browser_launch_;
 
   // True if this is a multi-desktop test (in which case this browser test will
   // not ensure that Browsers are only created on the tested desktop).
   bool multi_desktop_test_;
 
+  // True if the accessibility tests should be run.
+  bool run_accessibility_checks_;
+
+  // True if the accessibility test should run for a particular test case.
+  // This is reset for every test case.
+  bool run_accessibility_checks_for_test_case_;
+
 #if defined(OS_MACOSX)
   base::mac::ScopedNSAutoreleasePool* autorelease_pool_;
 #endif  // OS_MACOSX
 
 #if defined(OS_WIN)
   scoped_ptr<base::win::ScopedCOMInitializer> com_initializer_;
 #endif
 };
 
 #endif  // CHROME_TEST_BASE_IN_PROCESS_BROWSER_TEST_H_