Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openshift-installer-presubmits: Use TEST_SKIP for e2e-aws #1341

Closed
wants to merge 1 commit into from

Commits on Sep 4, 2018

  1. openshift-installer-presubmits: Use TEST_SKIP for e2e-aws-all

    I want to improve our e2e-aws coverage by blacklisting known failures.
    I generated this string from an e2e-aws-all run today against the
    master codebase [1,2] with this helper script:
    
      $ cat ~/bin/junit-blacklist.py
      #!/usr/bin/env python3
    
      import os
      import re
      import sys
      from xml.etree.ElementTree import ElementTree
    
      def get_failures(stream=sys.stdin):
          tree = ElementTree()
          tree.parse(stream)
          for test in tree.findall('testcase'):
              if test.find('skipped') is not None:
                  continue
              elif test.find('failure') is not None:
                  print('failure: {}'.format(test.attrib['name']))
                  yield test.attrib['name']
              else:
                  print('success: {}'.format(test.attrib['name']))
    
      def get_all_failures(root=os.curdir):
          for path, dirnames, filenames in os.walk(root):
              for filename in filenames:
                  if filename.endswith('.xml'):
                      yield from get_failures(stream=open(os.path.join(path, filename)))
    
      def process_failure(failure):
          chars = []
          depth = 0
          for char in failure:
              if char == '[':
                  depth += 1
                  continue
              if char == ']':
                  depth -= 1
                  continue
              if depth == 0:
                  chars.append(char)
          processed = re.escape(''.join(chars).strip())
          processed = processed.replace(r'\ ', ' ').replace(r'\-', '-').replace(r'\/', '/').replace(r'\=', '=')  # Work around https://bugs.python.org/issue29995, fixed in Python 3.7
          return processed
    
      if __name__ == '__main__':
          failures = sorted(set(process_failure(failure=failure) for failure in get_all_failures()))
          if len(failures) == 1:
              print(failures[0])
          elif failures:
              print('>-')
              print('.*(({}'.format(failures[0]))
              for failure in failures[1:]:
                  print('*)|({}'.format(failure))
              print('*)).*')
    
    and this invocation:
    
      $ wget -r -e robots=off -np -H https://gcsweb-ci.svc.ci.openshift.org/gcs/origin-ci-test/pr-logs/pull/openshift_installer/170/pull-ci-origin-installer-e2e-aws-all/4/artifacts/e2e-aws-all/junit/
      $ cd storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/170/pull-ci-origin-installer-e2e-aws-all/3/artifacts/e2e-aws-all/junit
      $ junit-blacklist.py
    
    The regexp is because Ginko's -skip takes a regexp [3].  Presumably
    that's a Go regexp [4], which, like Python regexps [5], are similar to
    POSIX's Extended Regular Expressions (EREs) [6].  From the JUnit XML,
    the script collects names for all failed tests and assembles a regexp
    to ignore all of them.  My target regexp pattern is:
    
      .*(({test-1-name})|({test-2-name})|({test-3-name})).*
    
    but that would get a bit hard to read.  So instead, I tried injecting
    optional whitespace after each name:
    
      .*(({test-1-name} *)|({test-2-name} *)|({test-3-name} *)).*
    
    and then using YAML's folded style [7] with the stripping indicator
    [8] to get:
    
      >-
      .*(({test-1-name}
      *)|({test-2-name}
      *)|({test-3-name}
      *)).*
    
    As we improve the installer, we'd gradually remove entries from this
    blacklist by cutting a single line from the folded regexp.  The
    ordered-prow-config test didn't like my line wrapping, though [9]:
    
      --- /tmp/tmp.hlKAjJ4Mf4/jobs/openshift/installer/openshift-installer-presubmits.yaml  2018-08-31 23:26:44.138011998 +0000
      +++ hack/../ci-operator/jobs/openshift/installer/openshift-installer-presubmits.yaml  2018-08-31 23:26:44.495040384 +0000
      @@ -113,53 +111,8 @@ presubmits:
               - name: TEST_FOCUS_SERIAL
                 value: Suite:openshift/registry/serial
               - name: TEST_SKIP
      -          value: >
      -            .*((DNS should answer endpoint and wildcard queries for the cluster
      -            ...
      -            *)).*
      +          value: |
      +            .*((DNS should answer endpoint and wildcard queries for the cluster...*)).*
    
    and it didn't like my attempt to use one long line (with stripping
    [8]) [10]:
    
      --- /tmp/tmp.kwtUW4jpNh/jobs/openshift/installer/openshift-installer-presubmits.yaml  2018-09-04 17:59:44.796949829 +0000
      +++ hack/../ci-operator/jobs/openshift/installer/openshift-installer-presubmits.yaml  2018-09-04 17:59:44.910959930 +0000
      @@ -113,8 +111,60 @@ presubmits:
               - name: TEST_FOCUS_SERIAL
                 value: Suite:openshift/registry/serial
               - name: TEST_SKIP
      -          value: |-
      -            .*((DNS should answer endpoint and wildcard queries for the cluster)|...).*
      +          value: .*((DNS should answer endpoint and wildcard queries for the cluster)|(HostPath
      +            should support subPath)|(Image hard prune should delete orphaned blobs)|(Image
      +            ...
      +            pass security tests)).*
    
    so with this commit I've just used the form with the test-required
    wrapping (without the optional whitespace hack).
    
    This change is fairly dramatic, so I'm temporarily hijacking
    e2e-aws-all to exercise it.  If it works well there, I'll move this
    TEST_SKIP into e2e-aws and restore e2e-aws-all to its previous setup.
    
    Keeping the FOCUS variables keeps the tests from expanding into
    untested waters [11].  If we want to drop filters later, we'd need a
    new run to figure out what would need blacklisting in the expanded
    suite.
    
    [1]: openshift/installer#170 (comment)
    [2]: https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/openshift_installer/170/pull-ci-origin-installer-e2e-aws-all/3/
    [3]: https://github.com/onsi/ginkgo/blame/v1.6.0/README.md#L30
    [4]: https://golang.org/pkg/regexp/#pkg-overview
    [5]: https://docs.python.org/3/library/re.html#regular-expression-syntax
    [6]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_04
    [7]: http://yaml.org/spec/1.2/spec.html#style/block/folded
    [8]: http://yaml.org/spec/1.2/spec.html#id2794534
    [9]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_release/1341/pull-ci-openshift-release-master-ordered-prow-config/111/build-log.txt
    [10]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_release/1341/pull-ci-openshift-release-master-ordered-prow-config/137/build-log.txt
    [11]: openshift#1341 (comment)
    wking committed Sep 4, 2018
    Configuration menu
    Copy the full SHA
    546bfdf View commit details
    Browse the repository at this point in the history