Hello CLI maintainers,
(TL;DR I'm tired of broken tests in hammer-cli-katello due to hammer-cli
changes)
We have a complex dependency structure between our CLI libraries
(hammer-cli, hammer-cli-foreman, hammer-cli-katello, csv, admin, etc.), and
it's not necessarily immune to changes in lower projects (i.e. hammer-cli).
An example is this recent PR (https://github.com/
theforeman/hammer-cli/pull/233) that slightly changes the
options/all_options methods in the abstract command. This caused failures
in hammer-cli-katello where we override/extend those methods, and I had to
open a PR to address the changes.
I'd like to start a discussion on ways to keep this from happening in the
future. Some solutions I've thought of include (1) mentioning people on PRs
that have the potential to break dependent projects and (2) running
dependent projects' tests when a PR is submitted.
(1) is not very robust. People can easily forget to mention others, and
it's difficult to remember all the appropriate people to mention.
(2) seems like more of a pain for maintainers of the lower projects
(hammer-cli). If we went with this, it would still be up to the dependent
projects to update to adapt to the changes in hammer-cli. It has the
benefit of immediately alerting the maintainers that a breaking change is
incoming. Overall, the hammer tests run in seconds; I think
hammer-cli-foreman is the longest-running at ~120 seconds. hammer-cli runs
in less than a second, and hammer-cli-katello runs in about 5 seconds.
Let me know what you think!
···
--
Andrew Kofink
akofink@redhat.com
IRC: akofink
Associate Software Engineer
Red Hat Satellite
Can we get the tests into bats instead of non-live-server? I think that's
the only way issues with authorization, etc. are going to be found. I agree
all the modules should be part of the suite.
···
On Wed, Mar 29, 2017 at 5:48 PM, Andrew Kofink wrote:
Hello CLI maintainers,
(TL;DR I’m tired of broken tests in hammer-cli-katello due to hammer-cli
changes)
We have a complex dependency structure between our CLI libraries
(hammer-cli, hammer-cli-foreman, hammer-cli-katello, csv, admin, etc.), and
it’s not necessarily immune to changes in lower projects (i.e. hammer-cli).
An example is this recent PR (https://github.com/theforeman
/hammer-cli/pull/233) that slightly changes the options/all_options
methods in the abstract command. This caused failures in hammer-cli-katello
where we override/extend those methods, and I had to open a PR to address
the changes.
I’d like to start a discussion on ways to keep this from happening in the
future. Some solutions I’ve thought of include (1) mentioning people on PRs
that have the potential to break dependent projects and (2) running
dependent projects’ tests when a PR is submitted.
(1) is not very robust. People can easily forget to mention others, and
it’s difficult to remember all the appropriate people to mention.
(2) seems like more of a pain for maintainers of the lower projects
(hammer-cli). If we went with this, it would still be up to the dependent
projects to update to adapt to the changes in hammer-cli. It has the
benefit of immediately alerting the maintainers that a breaking change is
incoming. Overall, the hammer tests run in seconds; I think
hammer-cli-foreman is the longest-running at ~120 seconds. hammer-cli runs
in less than a second, and hammer-cli-katello runs in about 5 seconds.
Let me know what you think!
–
Andrew Kofink
akofink@redhat.com
IRC: akofink
Associate Software Engineer
Red Hat Satellite
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
> Hello CLI maintainers,
>
> (TL;DR I'm tired of broken tests in hammer-cli-katello due to hammer-cli
> changes)
>
> We have a complex dependency structure between our CLI libraries
> (hammer-cli, hammer-cli-foreman, hammer-cli-katello, csv, admin, etc.), and
> it's not necessarily immune to changes in lower projects (i.e. hammer-cli).
> An example is this recent PR
> (https://github.com/theforeman/hammer-cli/pull/233) that slightly changes
> the options/all_options methods in the abstract command. This caused
> failures in hammer-cli-katello where we override/extend those methods, and I
> had to open a PR to address the changes.
>
> I'd like to start a discussion on ways to keep this from happening in the
> future. Some solutions I've thought of include (1) mentioning people on PRs
> that have the potential to break dependent projects and (2) running
> dependent projects' tests when a PR is submitted.
>
> (1) is not very robust. People can easily forget to mention others, and it's
> difficult to remember all the appropriate people to mention.
>
I'm afraid this isn't very robust indeed. I usually send a PR into
plugins when I discover the change could break them. The problem is in
the other cases when you think your change is fine (like the last
time, I didn't expect hammer_cli_katello would be overriding the
all_options method).
> (2) seems like more of a pain for maintainers of the lower projects
> (hammer-cli). If we went with this, it would still be up to the dependent
> projects to update to adapt to the changes in hammer-cli. It has the benefit
> of immediately alerting the maintainers that a breaking change is incoming.
> Overall, the hammer tests run in seconds; I think hammer-cli-foreman is the
> longest-running at ~120 seconds. hammer-cli runs in less than a second, and
> hammer-cli-katello runs in about 5 seconds.
I agree that adding the plugin tests would be good but they should be
a warning that you changed something and you should notify plugin
maintainers/consider sending patch rather than error that would block
the merge forever.
···
On Wed, Mar 29, 2017 at 11:48 PM, Andrew Kofink wrote:
Let me know what you think!
–
Andrew Kofink
akofink@redhat.com
IRC: akofink
Associate Software Engineer
Red Hat Satellite
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
