content/browser/accessibility/dump_accessibility_tree_browsertest.cc - Issue 9639016: Adds ability to comment expected files in DumpAccessibilityTests.

Side by Side Diff: content/browser/accessibility/dump_accessibility_tree_browsertest.cc

Issue 9639016: Adds ability to comment expected files in DumpAccessibilityTests. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Initial patch. Created 8 years, 9 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 | Annotate | Revision Log

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include <string>	5 #include <string>

	6 #include <vector>

6	7

7 #include "base/logging.h"	8 #include "base/logging.h"

8 #include "base/path_service.h"	9 #include "base/path_service.h"

9 #include "base/string_util.h"	10 #include "base/string_util.h"

10 #include "base/string16.h"	11 #include "base/string16.h"

11 #include "base/utf_string_conversions.h"	12 #include "base/utf_string_conversions.h"

12 #include "chrome/browser/ui/browser.h"	13 #include "chrome/browser/ui/browser.h"

13 #include "chrome/test/base/in_process_browser_test.h"	14 #include "chrome/test/base/in_process_browser_test.h"

14 #include "chrome/test/base/ui_test_utils.h"	15 #include "chrome/test/base/ui_test_utils.h"

15 #include "content/browser/accessibility/browser_accessibility.h"	16 #include "content/browser/accessibility/browser_accessibility.h"

16 #include "content/browser/accessibility/browser_accessibility_manager.h"	17 #include "content/browser/accessibility/browser_accessibility_manager.h"

17 #include "content/browser/accessibility/dump_accessibility_tree_helper.h"	18 #include "content/browser/accessibility/dump_accessibility_tree_helper.h"

18 #include "content/browser/renderer_host/render_view_host_impl.h"	19 #include "content/browser/renderer_host/render_view_host_impl.h"

19 #include "content/public/browser/notification_service.h"	20 #include "content/public/browser/notification_service.h"

20 #include "content/public/browser/notification_types.h"	21 #include "content/public/browser/notification_types.h"

21 #include "content/public/browser/render_widget_host_view.h"	22 #include "content/public/browser/render_widget_host_view.h"

22 #include "content/public/browser/web_contents.h"	23 #include "content/public/browser/web_contents.h"

23 #include "content/public/common/content_paths.h"	24 #include "content/public/common/content_paths.h"

24 #include "testing/gtest/include/gtest/gtest.h"	25 #include "testing/gtest/include/gtest/gtest.h"

25	26

26 using content::OpenURLParams;	27 using content::OpenURLParams;

27 using content::RenderViewHostImpl;	28 using content::RenderViewHostImpl;

28 using content::RenderWidgetHostImpl;	29 using content::RenderWidgetHostImpl;

29 using content::RenderWidgetHost;	30 using content::RenderWidgetHost;

30 using content::Referrer;	31 using content::Referrer;

31	32

32 namespace {	33 namespace {

33 // Required to enter html content into a url.	34 // Required to enter html content into a url.

34 static const std::string kUrlPreamble = "data:text/html,\n<!doctype html>";	35 static const std::string kUrlPreamble = "data:text/html,\n<!doctype html>";

35 } // namespace	36 static const char kCommentToken = '#';

36	37

37 // This test takes a snapshot of the platform BrowserAccessibility tree and	38 // This test takes a snapshot of the platform BrowserAccessibility tree and

38 // tests it against an expected baseline.	39 // tests it against an expected baseline.

39 //	40 //

40 // The flow of the test is as outlined below.	41 // The flow of the test is as outlined below.

41 // 1. Load an html file from chrome/test/data/accessibility.	42 // 1. Load an html file from chrome/test/data/accessibility.

42 // 2. Read the expectation.	43 // 2. Read the expectation.

43 // 3. Browse to the page and serialize the platform specific tree into a human	44 // 3. Browse to the page and serialize the platform specific tree into a human

44 // readable string.	45 // readable string.

45 // 4. Perform a comparison between actual and expected and fail if they do not	46 // 4. Perform a comparison between actual and expected and fail if they do not

46 // exactly match.	47 // exactly match.

47 class DumpAccessibilityTreeTest : public InProcessBrowserTest {	48 class DumpAccessibilityTreeTest : public InProcessBrowserTest {

48 public:	49 public:

	50 // Utility helper that does a comment aware equality check.

	51 void ExpectEqualsWithComments(std::string& expected, std::string& actual) {

	52 std::vector<std::string> actual_lines, expected_lines;

	53 std::string line_ending = UTF16ToUTF8(helper_.GetLineEnding());

	54 int actual_line_length = Tokenize(actual, line_ending, &actual_lines);
	dmazzoni 2012/03/09 19:55:37 How about line_count instead of line_length? line_ How about line_count instead of line_length? line_length makes me think of the number of characters in a line, but here it means the number of lines. David Tseng 2012/03/12 18:51:58 substituted with lines_count. On 2012/03/09 19: substituted with lines_count. On 2012/03/09 19:55:37, Dominic Mazzoni wrote: Show quoted text > How about line_count instead of line_length? line_length makes me think of the > number of characters in a line, but here it means the number of lines.
	55 int expected_line_length = Tokenize(expected, line_ending, &expected_lines);

	56 int i = actual_line_length - 1, j = expected_line_length - 1;

	57 while (i >= 0 && j >= 0) {

	58 if (expected_lines[j].size() > 0 &&

	59 expected_lines[j][0] == kCommentToken) {

	60 --j;

	61 continue;

	62 }

	63

	64 EXPECT_EQ(actual_lines[i], expected_lines[j]);
	dmazzoni 2012/03/09 19:55:37 This could spew a whole bunch of errors if there's This could spew a whole bunch of errors if there's a difference. It also might be confusing to have them appear in reverse order like this. How about just setting a flag and then expect just one at the end? Alternatively maybe it'd be more clear to have this function return a boolean and do the EXPECT from the calling function? That might work better with my merge. David Tseng 2012/03/12 18:51:58 Returns result of comparison now. On 2012/03/09 19 Returns result of comparison now. On 2012/03/09 19:55:37, Dominic Mazzoni wrote: Show quoted text > This could spew a whole bunch of errors if there's a difference. It also might > be confusing to have them appear in reverse order like this. > > How about just setting a flag and then expect just one at the end? > > Alternatively maybe it'd be more clear to have this function return a boolean > and do the EXPECT from the calling function? That might work better with my > merge. >
	65 --i;

	66 --j;

	67 }

	68

	69 // Actual file has been fully checked.

	70 EXPECT_LT(i, 0);

	71 }

	72

49 DumpAccessibilityTreeHelper helper_;	73 DumpAccessibilityTreeHelper helper_;

50 };	74 };

51	75

52 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,	76 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,

53 PlatformTreeDifferenceTest) {	77 PlatformTreeDifferenceTest) {

54 content::RenderWidgetHostView* host_view =	78 content::RenderWidgetHostView* host_view =

55 browser()->GetSelectedWebContents()->GetRenderWidgetHostView();	79 browser()->GetSelectedWebContents()->GetRenderWidgetHostView();

56 RenderWidgetHost* host = host_view->GetRenderWidgetHost();	80 RenderWidgetHost* host = host_view->GetRenderWidgetHost();

57 // TODO(joi): Remove this dependency	81 // TODO(joi): Remove this dependency

58 RenderViewHostImpl* view_host =	82 RenderViewHostImpl* view_host =

(...skipping 41 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
100	124

101 // Wait for the tree.	125 // Wait for the tree.

102 tree_updated_observer.Wait();	126 tree_updated_observer.Wait();

103	127

104 // Perform a diff (or write the initial baseline).	128 // Perform a diff (or write the initial baseline).

105 string16 actual_contents;	129 string16 actual_contents;

106 helper_.DumpAccessibilityTree(	130 helper_.DumpAccessibilityTree(

107 host_view->GetBrowserAccessibilityManager()->GetRoot(),	131 host_view->GetBrowserAccessibilityManager()->GetRoot(),

108 &actual_contents);	132 &actual_contents);

109 std::string actual_contents8 = UTF16ToUTF8(actual_contents);	133 std::string actual_contents8 = UTF16ToUTF8(actual_contents);

110 EXPECT_EQ(expected_contents, actual_contents8);	134 ExpectEqualsWithComments(expected_contents, actual_contents8);

111	135

112 if (!file_util::PathExists(expected_file)) {	136 if (!file_util::PathExists(expected_file)) {

113 FilePath actual_file =	137 FilePath actual_file =

114 FilePath(html_file.RemoveExtension().value() +	138 FilePath(html_file.RemoveExtension().value() +

115 helper_.GetActualFileSuffix());	139 helper_.GetActualFileSuffix());

116	140

117 EXPECT_TRUE(file_util::WriteFile(	141 EXPECT_TRUE(file_util::WriteFile(

118 actual_file, actual_contents8.c_str(), actual_contents8.size()));	142 actual_file, actual_contents8.c_str(), actual_contents8.size()));

119	143

120 ADD_FAILURE() << "No expectation found. Create it by doing:\n"	144 ADD_FAILURE() << "No expectation found. Create it by doing:\n"

121 << "mv " << actual_file.LossyDisplayName() << " "	145 << "mv " << actual_file.LossyDisplayName() << " "

122 << expected_file.LossyDisplayName();	146 << expected_file.LossyDisplayName();

123 }	147 }

124 } while (!(html_file = file_enumerator.Next()).empty());	148 } while (!(html_file = file_enumerator.Next()).empty());

125 }	149 }

	150

	151 } // namespace
	dmazzoni 2012/03/09 19:55:37 I think tests aren't supposed to be in an anonymou I think tests aren't supposed to be in an anonymous namespace - it prevents them from being selectable via gtest_filter. Let me know if you found examples to the contrary for this same type of test, or if there's another good reason. David Tseng 2012/03/12 18:51:58 gtest_filter works fine for me with the tests wrap gtest_filter works fine for me with the tests wrapped in a namespace. Saw this pattern being used elsewhere, but don't have a good reason beyond that. On 2012/03/09 19:55:37, Dominic Mazzoni wrote: Show quoted text > I think tests aren't supposed to be in an anonymous namespace - it prevents them > from being selectable via gtest_filter. Let me know if you found examples to the > contrary for this same type of test, or if there's another good reason.
OLD	NEW

« no previous file with comments | « no previous file | content/test/data/accessibility/aria-application-expected-win.txt » ('j') | no next file with comments »