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

Blacklist bears from CI #1476

Closed
jayvdb opened this issue Mar 4, 2017 · 10 comments
Closed

Blacklist bears from CI #1476

jayvdb opened this issue Mar 4, 2017 · 10 comments

Comments

@jayvdb
Copy link
Member

jayvdb commented Mar 4, 2017

With many bears in one repo, we regularly have CI breakages that effectively put a halt to all other development. At least nothing can get merged, and then other breakages occur and are not easily spotted.

The tree needs to stay green.

A particularly difficult open case is #1472 (gotype deleted).
But in the last week we've also had a four day red period due to #1461 (checkstyle jar corruption) , and while that was broken #1470 (ramllint breakage) also occurred but wasnt noticed and fixed promptly.

To avoid these prolonged red tree periods easily, we need to disable broken bears from all CI if they are not able to be fixed immediately.

Ideally this flagged status appears in the Bear code itself, so that the brokenness can also be reported to users. In most cases, if a bear is broken on our CI, it is very likely users will also be seeing the same problems. The only time this isnt true is if it is a Linux problem with our trusty CI workers that doesnt occur on more recent Ubuntu or other distros.

@satwikkansal
Copy link
Member

@jayvdb

One possible way I can think of doing so is,

  1. Add BLACKLIST flag in the bear's implementation.
  2. Define commands to install CI dependencies for a bear in a set name Install_dependencies
  3. Create a template for deps.sh file.
  4. Create a script for generating deps.sh using the template file and the above flags specified in the bears' implementaions. ( Taking care of blacklist status of the bear and also avoiding duplicate commands to install same dependency among different bears)
  5. Execute this script in the CI and then install dependencies from deps.sh.

@Makman2
Copy link
Member

Makman2 commented Mar 4, 2017

  1. I would call it BLACKLISTED :)
  2. If users use the stable version, they won't get notified via the BLACKLIST flag or similar.

@jayvdb
Copy link
Member Author

jayvdb commented Mar 5, 2017

@satwikkansal , the requirements aspect of your post is a good medium term goal, but beyond scope of this issue. There is already quite a bit of work done in #1386 , It depends on a lot more effort occurring in the dependency_management repo.

If you take a look at the current problem (#1472), a generated installation routine will fail unless it is very complex. It would need a special DeprecatedGoTypeRequirement to cater for that specific problem.

Hence we need blacklisting of some bears.
And it may be a transient problem.
Sometimes the problem fixes itself upstream, so rechecking the blacklist near release could be the best strategy. e.g. #1470 needed a particular upstream release to be backlisted, but the upstream have now fixed the problem and done a new release, and marked the problematic release as deprecated, so it is unlikely to affect anyone.

@coala-bot coala-bot assigned yash-nisar and unassigned yash-nisar Mar 30, 2017
@jayvdb
Copy link
Member Author

jayvdb commented May 28, 2017

We've just encountered a significant problem in requests coala/coala#4277 (comment) which could be fixed in vint-vim, but could also be fixed by blacklisting VintBear . Just blacklisting it from CI would need to also allow it to be skipped by the CI in the coala repo.

That means the blacklisting needs to drop the requirements from the 'pre-release', so it can be installed by the CI in the coala repo.

@Vamshi99
Copy link
Member

@jayvdb If we want any code to be skipped by CI, why don't we make the code as a comment, till the issue get's fixed. This way our code also remains safe :)

@meetmangukiya
Copy link
Member

meetmangukiya commented May 28, 2017

I think we want dynamic blacklisting from CI, and not commit the blacklisting to master.

Similar to how we blacklist gitcommitbear from CI coafile

@Vamshi99
Copy link
Member

In that case, bear is just not being run, but in here we have a problem with dependencies, so it will not be similar @meetmangukiya

@jayvdb
Copy link
Member Author

jayvdb commented May 28, 2017

The blacklisting should be part of coala:

Ideally this flagged status appears in the Bear code itself, so that the brokenness can also be reported to users.

We need to ship/release new versions with some bears disabled when we have 'unfixable' problems.

@Vamshi99
Copy link
Member

So, are we just going to disable the bear in coafile, I don't think that will be a good option. I had prefer making it as comment 😕

@jayvdb
Copy link
Member Author

jayvdb commented Aug 4, 2019

This is now possible by custom rules in .ci/get_tests.py, which was added so that we could reach 100% coverage on AppVeyor while so many bears are still broken or not easily installable on Windows.

@jayvdb jayvdb closed this as completed Aug 4, 2019
jayvdb added a commit to jayvdb/coala-bears that referenced this issue Aug 4, 2019
Custom rules in tox.ini are migrated into get_tests.py so
that they can be dynamically collated together.

Disable phpmd test test_cleancode_violation, as it is currently
failing due to a minor text change in the error emitted by the
linter.

Related to coala#2943
Related to coala#1476
jayvdb added a commit to jayvdb/coala-bears that referenced this issue Aug 5, 2019
Custom rules in tox.ini are migrated into get_tests.py so
that they can be dynamically collated together.

Disable phpmd test test_cleancode_violation, as it is currently
failing due to a minor text change in the error emitted by the
linter.

Related to coala#2943
Related to coala#1476
jayvdb added a commit to jayvdb/coala-bears that referenced this issue Aug 6, 2019
Custom rules in tox.ini are migrated into get_tests.py so
that they can be dynamically collated together.

Related to coala#1476
jayvdb added a commit to jayvdb/coala-bears that referenced this issue Aug 6, 2019
Custom rules in tox.ini are migrated into get_tests.py so
that they can be documented and dynamically collated together.

Related to coala#1476
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants