Skip to content
This repository was archived by the owner on Feb 5, 2020. It is now read-only.

tests: Report specific smoke test status to GitHub#2082

Merged
mxinden merged 6 commits intocoreos:masterfrom
mxinden:report-status-github
Nov 1, 2017
Merged

tests: Report specific smoke test status to GitHub#2082
mxinden merged 6 commits intocoreos:masterfrom
mxinden:report-status-github

Conversation

@mxinden
Copy link
Contributor

@mxinden mxinden commented Oct 11, 2017

No description provided.

@mxinden mxinden force-pushed the report-status-github branch 11 times, most recently from 1f0527d to 37037ff Compare October 11, 2017 13:25
@mxinden mxinden changed the title [WIP] tests: Report specific smoke test status to GitHub tests: Report specific smoke test status to GitHub Oct 11, 2017
@mxinden mxinden requested a review from cpanato October 11, 2017 13:25
@mxinden
Copy link
Contributor Author

mxinden commented Oct 16, 2017

@cpanato Could you review this?

@mxinden mxinden force-pushed the report-status-github branch from 37037ff to d3e1166 Compare October 16, 2017 07:47
cpanato
cpanato previously approved these changes Oct 16, 2017
@cpanato
Copy link
Contributor

cpanato commented Oct 16, 2017

retest this please

@cpanato cpanato closed this Oct 16, 2017
@cpanato cpanato reopened this Oct 16, 2017
@mxinden mxinden force-pushed the report-status-github branch 2 times, most recently from 6c50bcd to 93536e6 Compare October 16, 2017 08:53
@cpanato
Copy link
Contributor

cpanato commented Oct 16, 2017

@mxinden when you click in the details for each feedback it redirects to https://example.com/build/status and not to our Jenkins. Can you please take a look on that?

@mxinden mxinden force-pushed the report-status-github branch 2 times, most recently from 1ebf015 to af46844 Compare October 17, 2017 07:02
@mxinden
Copy link
Contributor Author

mxinden commented Oct 17, 2017

@sym3tri In the status field of this PR you can see the detailed Jenkins status feedback. Does this look good to you?

@mxinden mxinden force-pushed the report-status-github branch 5 times, most recently from e47459d to 50b31d0 Compare October 30, 2017 22:35
@mxinden
Copy link
Contributor Author

mxinden commented Oct 31, 2017

@cpanato Could you give this another review? I fixed the stashing issue by reducing the scope of the stashing to only those files necessary (See 50b31d0). Let me know if this looks good to you.

Instead of reporting overall status to Github, this patch introduces
more detailed status reports:

1. `basic-tests` includes all basic tests in the first stage of our
pipeline. This status will be required to be green. In case it is red, a
merge of the PR is disabled.
2. `gui-tests` represents the result of the GUI tests stage. It is not
mendatory for merge. It will only run when the `run-gui-tests` label is
set.
3. `smoke-tests/<test-name>` is going to be reported per smoke test.
None of these status reports are mandatory for merge. These tests are
only run when the `run-smoke-tests` label is set.
4. `continuous-integration/jenkins/pr-merge` is the overall status of
the entire pipeline. This is not mandatory. I am looking into
deactivating this in total, but haven't found a solution yet.
According to the Jenkins documentation `stash` should only be used for
small amounts of data [1]. To prevent `java.io.IOException: Failed to
extract repository.tar.gz` this patch reduces the stash scope to the
npm_modules, the installer binary and the smoke test binary. The
remaining repository files are loaded via standard git checkouts.

[1]
https://jenkins.io/doc/pipeline/steps/workflow-basic-steps/#code-stash-code-stash-some-files-to-be-used-later-in-the-build
@mxinden mxinden force-pushed the report-status-github branch from 50b31d0 to 8bd13bb Compare October 31, 2017 11:35
@cpanato
Copy link
Contributor

cpanato commented Oct 31, 2017

retest this please

Copy link
Contributor

@cpanato cpanato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I just have one concern to do the checkout in all blocks.

@mxinden
Copy link
Contributor Author

mxinden commented Oct 31, 2017

@cpanato What is the concern?

In case you are worried about the delay introduced by this change: The last test execution of this PR was 45 minutes. Last master execution was 48 minutes.

@cpanato
Copy link
Contributor

cpanato commented Oct 31, 2017

@mxinden more in terms of doing the checkout every time during the build. but this will be fixed when we implement the new builder :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants