Skip to content

Commit

Permalink
openshift-installer-presubmits: Use TEST_SKIP for e2e-aws-all
Browse files Browse the repository at this point in the history
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]: #1341 (comment)
  • Loading branch information
wking committed Sep 4, 2018
1 parent 87efd64 commit 546bfdf
Showing 1 changed file with 11 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ presubmits:
value: Suite:openshift/conformance/parallel
- name: TEST_FOCUS_SERIAL
value: Suite:openshift/conformance/serial
- name: TEST_SKIP
value: .*((Image hard prune should delete orphaned blobs)|(Image hard prune
should show orphaned blob deletions in dry-run mode)|(Image limit range
should deny a docker image reference exceeding limit on openshift\.io/image-tags
resource)|(Image prune of schema 1 should prune old image)|(Image prune
of schema 2 should prune old image with config)|(Image prune with --all=false
flag should prune only internally managed images)|(Image prune with --prune-registry==false
should prune old image but skip registry)|(Image prune with default --all
flag should prune both internally managed and external images)|(Image
resource quota should deny a push of built image exceeding openshift\.io/imagestreams
quota)).*
- name: JOB_NAME_SAFE
value: e2e-aws-all
- name: CLUSTER_TYPE
Expand Down

0 comments on commit 546bfdf

Please sign in to comment.