Skip to content
This repository has been archived by the owner on Mar 22, 2021. It is now read-only.

Provide support for an overall status #18

Open
jlebon opened this issue Nov 22, 2016 · 13 comments
Open

Provide support for an overall status #18

jlebon opened this issue Nov 22, 2016 · 13 comments

Comments

@jlebon
Copy link
Collaborator

jlebon commented Nov 22, 2016

Some services like Homu watch for statuses to determine whether to merge a commit. In those cases, it might be easier for it to watch for a single "overall" status rather than multiple statuses, whose set may change over time.

@jlebon
Copy link
Collaborator Author

jlebon commented Nov 22, 2016

Tentative spec:

# OPTIONAL
# Specify the context group to which this testsuite belongs.
# This commit status context will be marked as successful
# only if all the member testsuites are successful. If
# omitted, no group context is defined.
context-group: 'Red Hat CI - Overall status'

For example, in the Homu use case, this would allow you to define the set of testsuites on which we want to gate (e.g. you might have a testsuite that is flaky/a work in progress).

@cgwalters
Copy link
Member

cgwalters commented Nov 22, 2016

Seems OK. Though I wonder if it's simpler to support rolling suites under a single context by default, like:

context: optional/test1 and context: optional/test2 would result in a single optional context that merged the two suites. Call them "context name" and "suite name". The suite name would then be a prefix in that case for uploaded artifacts etc.

The advantage would be less visual noise per PR.

Something like this:

branches:
    - master
    - auto
    - try

context: baseline/sanitize-undefined

container:
    image: projectatomic/ostree-tester

env:
    CFLAGS: '-fsanitize=undefined'

build:
    config-opts: >
      --prefix=/usr
      --libdir=/usr/lib64
      --enable-installed-tests
      --enable-gtk-doc

tests:
    - make syntax-check
    - make check
    - gnome-desktop-testing-runner ostree
    - sudo --user=testuser gnome-desktop-testing-runner ostree

timeout: 30m

artifacts:
    - test-suite.log

---

context: baseline/clang
inherit: true
env:
    CC: 'clang'
    CFLAGS: '-Werror=unused-variable'
tests:
artifacts:

---

inherit: true
context: optional/ASAN
# Until the container is regenerated
packages:
  - libasan

env:
    # Suppress leak detection in configure (and build, since
    # g-ir-scanner uses glib), because sadly some of them leak
    ASAN_OPTIONS: 'detect_leaks=false'
    CFLAGS: '-fsanitize=address -fno-omit-frame-pointer'

tests:
    - env OT_SKIP_READDIR_RAND=1 ASAN_OPTIONS='detect_leaks=true:malloc_context_size=20' make check

artifacts:
    - config.log
    - test-suite.log

@jlebon
Copy link
Collaborator Author

jlebon commented Nov 22, 2016

I like the idea of less visual noise and decoupling contexts from suites. Though I'd prefer if we were more explicit. How about we leave context to be the actual context, and have e.g. a suite key?

@cgwalters
Copy link
Member

So one would probably rely on inherit: true and have context: foo, then define multiple suites, then reset it with context: bar and further suites?

@cgwalters
Copy link
Member

How about name instead of suite?

@jlebon
Copy link
Collaborator Author

jlebon commented Nov 22, 2016

Yes exactly!

Sure, name is nicer.

@cgwalters
Copy link
Member

Am currently thinking for ostree naming the contexts like homu-required, and optional.

@cgwalters
Copy link
Member

And the suites like c7-build, asan, clang, etc.

@jlebon
Copy link
Collaborator Author

jlebon commented Nov 22, 2016

Thinking about this a bit more. The trade-off we're making here is that we get less feedback. E.g. within one context, if a suite is already done but others are still going, we'll get no feedback until all the suites are completed and the artifacts get uploaded.

I suppose what we'd have to do here is:

  • Upload a preliminary "summary" results page and just keep updating & re-uploading that whenever a suite terminates.
  • As soon as a suite fails, set the context to fail to provide feedback.

But also, what constitutes visual noise can be somewhat subjective. Since almost all the projects use homu, I'm guessing this required/optional grouping will be used by all of them?

Since we rely so much on homu, maybe it would make sense to have a homu-specific feature here. E.g. add a homu key which if true:

  • Adds auto and try to the list of branches to watch for
  • Pushes out e.g. a homu context on branch tests (no need to pollute PRs), which is set as successful only if all the suites with the homu key passed.

I'm not saying the context & name idea is bad, just trying to decouple the homu requirements from how it's presented to users, which may vary.

@cgwalters
Copy link
Member

I'm not so concerned about waiting for feedback myself, when using Homu one can approve before waiting for PR status, and I definitely do that. I'll be even more confident when we implement this and the context gates on multiple tests (like C7 and Fedora).

But long term, we'll clearly want some public facing server with dynamic logs, like Travis etc. do.

As far as adding more Homu features...probably yes, but I'm not sure it's the same thing as this issue. If we called the context required it feels more explanatory than homu I'd say.

@jlebon
Copy link
Collaborator Author

jlebon commented Nov 23, 2016

I'm not so concerned about waiting for feedback myself, when using Homu one can approve before waiting for PR status, and I definitely do that.

But that's a choice we should be able to make per-repository, right? E.g. I can see this making sense in the ostree case where we have multiple sanitizers, whereas on rpm-ostree I think it'd make more sense to keep check and vmcheck as separate contexts.

Essentially, GitHub already provides a nice way of presenting the state of different testsuites and accessing their logs, so let's make use of it when it makes sense, rather than having to partially re-implement it on S3.

As far as adding more Homu features...probably yes, but I'm not sure it's the same thing as this issue. If we called the context required it feels more explanatory than homu I'd say.

Right, definitely separate. What I mean here is, for a start, let's implement the required key so that we can at least transfer homu over to use Red Hat CI on all the projects. And we can keep this issue to discuss the context & name implementation. The former is trivial, the latter is trickier if we want to do it right.

What do you think?


Just to formalize the required idea a bit. Maybe something like:

# OPTIONAL
# Mark this testsuite as required. This causes a special
# "required" context to be reported to GitHub on branch
# tests. The result is set to successful only if all
# testsuites marked as required are also successful.
required: true

?

@cgwalters
Copy link
Member

required: true sounds good to me.

@jlebon
Copy link
Collaborator Author

jlebon commented Nov 23, 2016

#19

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants