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

travis: continue static tests on error #4948

Merged
merged 1 commit into from
Mar 4, 2016

Conversation

cgundogan
Copy link
Member

Continue static tests even if one of them fails. This way we might get more feedback in one run, instead of having a running - testing - running - testing loop.

In the current implementation the script exits as soon as one of static tests fail (because of set -e).

@cgundogan cgundogan added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: CI Area: Continuous Integration of RIOT components labels Mar 2, 2016
@OlegHahm
Copy link
Member

OlegHahm commented Mar 3, 2016

Not sure - @authmillenon, what do you think?

@cgundogan
Copy link
Member Author

what's the benefit of not running cppcheck if the doxycheck failed beforehand? (:

@OlegHahm
Copy link
Member

OlegHahm commented Mar 3, 2016

Time.

@cgundogan
Copy link
Member Author

I disagree. IMO, it is much more time efficient to run them all in one slide instead of waiting for travis to enqueue the job again.

@miri64
Copy link
Member

miri64 commented Mar 3, 2016

Actually this is how this script is supposed to work. I do not know why the old solution did not work out (though I tried to debug multiple times). We don't save time if Travis is nitpicking over each time about a different thing and it's frustrating for new contributors. Just one question: what if the result is neither one nor zero? Is this also catched?

@OlegHahm
Copy link
Member

OlegHahm commented Mar 3, 2016

Convinced.

@miri64
Copy link
Member

miri64 commented Mar 3, 2016

(I don't know how trap is working)

@cgundogan
Copy link
Member Author

Actually this is how this script is supposed to work. I do not know why the old solution did not work out

@authmillenon the problem is that set -e exits the whole script once a non-zero exit code was encountered.

Just one question: what if the result is neither one nor zero? Is this also catched?
(I don't know how trap is working)

trap on ERR will be fired for each non-zero exit code. And I just set Result to 1 in that case

@miri64
Copy link
Member

miri64 commented Mar 3, 2016

Then ACK.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 3, 2016
miri64 added a commit that referenced this pull request Mar 4, 2016
@miri64 miri64 merged commit a7487e2 into RIOT-OS:master Mar 4, 2016
@cgundogan cgundogan deleted the pr/travis/static-tests_fix branch March 19, 2016 22:12
@kaspar030
Copy link
Contributor

Can someone confirm that this PR didn't break the logic? Murdock doesn't seem to exit on doc errors anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI Area: Continuous Integration of RIOT components CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants