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

[test] Fail if coverage can't be push #13084

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 3, 2018

The coverage report could previously fail silently which could leave codecov in an infinite pending state. If the report fails now CI will also fail. Since this didn't test anything that test:unit didn't test already I moved this to another job so that it doesn't block other jobs because of a simple network error.

See codecov/codecov-bash#134

eps1lon added a commit to eps1lon/material-ui that referenced this pull request Oct 3, 2018
@eps1lon eps1lon force-pushed the ci/explicit-codecov-fail branch from a1594e4 to 46d4bd6 Compare October 3, 2018 10:16
fi
- run:
name: Coverage
command: bash <(curl -s https://codecov.io/bash) -Z
Copy link
Member Author

Choose a reason for hiding this comment

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

-Z is the important change here.

@oliviertassinari
Copy link
Member

What do you think of keeping the existing build steps? Because running the regression tests is an intensive task, we only do it once all the other tests are green.

@eps1lon
Copy link
Member Author

eps1lon commented Oct 3, 2018

@oliviertassinari I only looked at the CI runtime but test_regression will also trigger argos, right? I will add test_coverage to the requirements for test_converage.

@oliviertassinari
Copy link
Member

Can we keep it into test_unit?

capture d ecran 2018-10-03 a 15 34 58

We only have 4 containers that can be run in parallel. Adding a fith step will slow the builds. Don't worry, CircleCi has already been optimized.

@eps1lon
Copy link
Member Author

eps1lon commented Oct 3, 2018

Sure, better keep it simple

The coverage report could previously fail silently which could leave
codecov in an indeterminate pending state. If the report fails
now CI will also fail.
@eps1lon eps1lon force-pushed the ci/explicit-codecov-fail branch from 3e6a9af to fc1e45a Compare October 3, 2018 13:42
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I have no clue what this option does, but I trust you with it. Do you have some documentation?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 3, 2018

I have no clue what this option does, but I trust you with it. Do you have some documentation?

Ok, found it in https://codecov.io/bash

-Z           Exit with 1 if not successful. Default will Exit with 0

@oliviertassinari oliviertassinari merged commit 97a0d37 into mui:master Oct 3, 2018
@oliviertassinari
Copy link
Member

Well done!

@eps1lon eps1lon deleted the ci/explicit-codecov-fail branch October 4, 2018 16:01
@oliviertassinari oliviertassinari changed the title [ci] Move coverage test to own job [test] Fail if coverage can't be push Oct 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants