blinkpy: Remove more code that changes expectation line numbers

Bug: 1050760, 1050754, 986447
Change-Id: Ib9af3b1b7994f312eb683fd66e60c67d4ef12aea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2193663
Commit-Queue: Rakib Hasan <rmhasan@google.com>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776757}

third_party/blink/tools/blinkpy/web_tests/models/test_expectations.py

@@ -222,16 +222,26 @@ class TestExpectations(object):
                 lines.extend(lineno_to_exps[lineno])
                 lineno_to_exps.pop(lineno)
 
-        # Add new expectations that were not part of the file before
+        # Handle Expectation instances with line numbers outside of the
+        # [1, total file line count] range. There are two cases for
+        # Expectation instances with line numbers outside the valid range.
+        #
+        # 1, If line number is 0 then the Expectation instance will be appended
+        #    to the file.
+        # 2, If the line number is greater than the total number of lines then
+        #    an exception will be raised.
         if lineno_to_exps:
             lines.append(_NotExpectation('', len(content_lines) + 1))
 
-        extra_line_count = len(content_lines) + 1
-        for lineno, extras in lineno_to_exps.items():
-            for line in extras:
+            for line in sorted(
+                    reduce(lambda x,y: x+y, lineno_to_exps.values()),
+                    key=lambda e: e.test):
+                if line.lineno:
+                    raise ValueError(
+                        "Expectation '%s' was given a line number that "
+                        "is greater than the total line count of file %s."
+                        % (line.to_string(), path))
                 lines.append(line)
-                extra_line_count += 1
-                lines[-1].lineno = extra_line_count
 
         self._expectation_file_linenos[path] = {
             line.lineno for line in lines
@@ -427,7 +437,7 @@ class TestExpectations(object):
             if not pattern_to_exps[exp.test]:
                 pattern_to_exps.pop(exp.test)
 
-    def add_expectations(self, path, exps, lineno=0):
+    def add_expectations(self, path, exps, lineno=0, append_to_end_of_file=False):
         """This method adds Expectation instances to an expectations file. It will
         add the new instances after the line number passed through the lineno parameter.
         If the lineno is set to a value outside the range of line numbers in the file
@@ -442,6 +452,14 @@ class TestExpectations(object):
         typ_expectations = self._expectations[idx]
         added_glob = False
 
+        if lineno < 0:
+            raise ValueError('lineno cannot be negative.')
+        if (append_to_end_of_file and lineno or
+                not append_to_end_of_file and not lineno):
+            raise ValueError('If append_to_end_of_file is set then lineno '
+                             'must be 0. Also if lineno is 0 then '
+                             'append_to_end_of_file must be set to True.')
+
         for exp in exps:
             exp.lineno = lineno
 

third_party/blink/tools/blinkpy/web_tests/models/test_expectations_unittest.py

@@ -32,8 +32,10 @@ import unittest
 
 from blinkpy.common.host_mock import MockHost
 from blinkpy.common.system.output_capture import OutputCapture
-from blinkpy.web_tests.models.test_configuration import TestConfiguration, TestConfigurationConverter
-from blinkpy.web_tests.models.test_expectations import TestExpectations, SystemConfigurationRemover, ParseError
+from blinkpy.web_tests.models.test_configuration import (
+    TestConfiguration, TestConfigurationConverter)
+from blinkpy.web_tests.models.test_expectations import (
+    TestExpectations, SystemConfigurationRemover, ParseError)
 from blinkpy.web_tests.models.typ_types import ResultType, Expectation
 
 
@@ -743,22 +745,136 @@ class RemoveExpectationsTest(Base):
 
 
 class AddExpectationsTest(Base):
-    def test_add_expectation(self):
+
+    def test_add_expectation_end_of_file_nonzero_lineno(self):
+        port = MockHost().port_factory.get('test-win-win7')
+        raw_expectations = ('# tags: [ Mac Win ]\n'
+                            '# tags: [ release ]\n'
+                            '# results: [ Failure ]\n'
+                            '\n'
+                            '# this is a block of expectations\n'
+                            'test [ failure ]\n')
+        expectations_dict = OrderedDict()
+        expectations_dict['/tmp/TestExpectations'] = ''
+        expectations_dict['/tmp/TestExpectations2'] = raw_expectations
+        test_expectations = TestExpectations(port, expectations_dict)
+
+        with self.assertRaises(ValueError) as ctx:
+            test_expectations.add_expectations(
+                '/tmp/TestExpectations2',
+                [Expectation(test='test3',
+                             results=set([ResultType.Failure]))],
+                lineno=0)
+            test_expectations.commit_changes()
+        self.assertIn('append_to_end_of_file must be set to True',
+                      str(ctx.exception))
+
+    def test_add_expectation_with_negative_lineno(self):
+        port = MockHost().port_factory.get('test-win-win7')
+        raw_expectations = ('# tags: [ Mac Win ]\n'
+                            '# tags: [ release ]\n'
+                            '# results: [ Failure ]\n'
+                            '\n'
+                            '# this is a block of expectations\n'
+                            'test [ failure ]\n')
+        expectations_dict = OrderedDict()
+        expectations_dict['/tmp/TestExpectations'] = ''
+        expectations_dict['/tmp/TestExpectations2'] = raw_expectations
+        test_expectations = TestExpectations(port, expectations_dict)
+
+        with self.assertRaises(ValueError) as ctx:
+            test_expectations.add_expectations(
+                '/tmp/TestExpectations2',
+                [Expectation(test='test3',
+                             results=set([ResultType.Failure]))],
+                lineno=-1)
+            test_expectations.commit_changes()
+        self.assertIn('cannot be negative', str(ctx.exception))
+
+    def test_add_expectation_outside_file_size_range(self):
+        port = MockHost().port_factory.get('test-win-win7')
+        raw_expectations = ('# tags: [ Mac Win ]\n'
+                            '# tags: [ release ]\n'
+                            '# results: [ Failure ]\n'
+                            '\n'
+                            '# this is a block of expectations\n'
+                            'test [ failure ]\n')
+        expectations_dict = OrderedDict()
+        expectations_dict['/tmp/TestExpectations'] = ''
+        expectations_dict['/tmp/TestExpectations2'] = raw_expectations
+        test_expectations = TestExpectations(port, expectations_dict)
+
+        with self.assertRaises(ValueError) as ctx:
+            test_expectations.add_expectations(
+                '/tmp/TestExpectations2',
+                [Expectation(test='test3',
+                             results=set([ResultType.Failure]))],
+                lineno=100)
+            test_expectations.commit_changes()
+        self.assertIn('greater than the total line count', str(ctx.exception))
+
+    def test_use_append_to_end_flag_non_zero_lineno(self):
+        # Use append_to_end_of_file=True with lineno != 0
+        # An exception should be raised.
         port = MockHost().port_factory.get('test-win-win7')
-        raw_expectations = ('# tags: [ Mac Win ]\n' '# results: [ Failure ]\n')
+        raw_expectations = ('# tags: [ Mac Win ]\n'
+                            '# tags: [ release ]\n'
+                            '# results: [ Failure ]\n'
+                            '\n'
+                            '# this is a block of expectations\n'
+                            'test [ failure ]\n')
+        expectations_dict = OrderedDict()
+        expectations_dict['/tmp/TestExpectations'] = ''
+        expectations_dict['/tmp/TestExpectations2'] = raw_expectations
+        test_expectations = TestExpectations(port, expectations_dict)
+
+        with self.assertRaises(ValueError) as ctx:
+            test_expectations.add_expectations(
+                '/tmp/TestExpectations2',
+                [Expectation(test='test3',
+                             results=set([ResultType.Failure]))],
+                lineno=100, append_to_end_of_file=True)
+            test_expectations.commit_changes()
+        self.assertIn('append_to_end_of_file is set then lineno must be 0',
+                      str(ctx.exception))
+
+    def test_add_expectations_to_end_of_file(self):
+        port = MockHost().port_factory.get('test-win-win7')
+        raw_expectations = ('# tags: [ Mac Win ]\n'
+                            '# tags: [ release ]\n'
+                            '# results: [ Failure ]\n'
+                            '\n'
+                            '# this is a block of expectations\n'
+                            'test [ failure ]\n')
         expectations_dict = OrderedDict()
         expectations_dict['/tmp/TestExpectations'] = ''
         expectations_dict['/tmp/TestExpectations2'] = raw_expectations
         test_expectations = TestExpectations(port, expectations_dict)
         test_expectations.add_expectations(
             '/tmp/TestExpectations2',
-            [Expectation(test='test1', results=set([ResultType.Failure]))])
+            [Expectation(test='test3', results=set([ResultType.Failure]))],
+            append_to_end_of_file=True)
+        test_expectations.add_expectations(
+            '/tmp/TestExpectations2',
+            [Expectation(test='test2', tags={'mac', 'release'},
+                         results={ResultType.Crash, ResultType.Failure})],
+            append_to_end_of_file=True)
+        test_expectations.add_expectations(
+            '/tmp/TestExpectations2',
+            [Expectation(test='test1', results=set([ResultType.Pass]))],
+            append_to_end_of_file=True)
         test_expectations.commit_changes()
         content = port.host.filesystem.read_text_file('/tmp/TestExpectations2')
         self.assertEqual(content, ('# tags: [ Mac Win ]\n'
+                                   '# tags: [ release ]\n'
                                    '# results: [ Failure ]\n'
                                    '\n'
-                                   'test1 [ Failure ]\n'))
+                                   '# this is a block of expectations\n'
+                                   'test [ failure ]\n'
+                                   '\n'
+                                   'test1 [ Pass ]\n'
+                                   '[ Release Mac ] test2 [ Failure Crash ]\n'
+                                   'test3 [ Failure ]\n'))
 
     def test_add_after_remove(self):
         port = MockHost().port_factory.get('test-win-win7')