Hi Gwen and Luke, Sorry, I’ve probably misunderstood something again. Since KAFKA-10155 and KAFKA-10147 have now been resolved, I merged the trunk back into my branch and added to comment “retest this please” to my pull request (https://github.com/apache/kafka/pull/8844 <https://github.com/apache/kafka/pull/8844>) like the contributing guidelines state. Unfortunately, no tests seem to have been run. Does a committer have to comment on the PR instead?
Thanks, Michael > On 16 Jun 2020, at 9:33 am, Michael Carter <michael.car...@instaclustr.com> > wrote: > > Great, thanks Luke. > I’ve undone the patch and added that comment. > > Cheers, > Michael > >> On 15 Jun 2020, at 6:07 pm, Luke Chen <show...@gmail.com> wrote: >> >> Hi Michael, >> The failed unit test has already handled here: >> https://issues.apache.org/jira/browse/KAFKA-10155 >> https://issues.apache.org/jira/browse/KAFKA-10147 >> >> So, maybe you can ignore the test errors and mention the issue number in PR. >> Thanks. >> >> Luke >> >> On Mon, Jun 15, 2020 at 3:23 PM Michael Carter < >> michael.car...@instaclustr.com> wrote: >> >>> Thanks for the response Gwen, that clarifies things for me. >>> >>> Regarding the unit test (ReassignPartitionsUnitTest. >>> testModifyBrokerThrottles), it appears to fail quite reliably on trunk as >>> well (at least on my machine). >>> It looks to me like a new override to >>> MockAdminClient.describeConfigs(Collection<ConfigResource> resources) >>> (MockAdminClient.java line 369) introduced in commit >>> 48b56e533b3ff22ae0e2cf7fcc649e7df19f2b06 changed the behaviour of this >>> method that the unit test relied on. >>> I’ve just now put a patch into my branch to make that test pass by calling >>> a slightly different version of describeConfigs (that avoids the overridden >>> behaviour). It’s probably arguable whether that constitutes a fix or not >>> though. >>> >>> Cheers, >>> Michael >>> >>>> On 15 Jun 2020, at 3:41 pm, Gwen Shapira <g...@confluent.io> wrote: >>>> >>>> Hi, >>>> >>>> 1. Unfortunately, you need to get a committer to approve running the >>> tests. >>>> I just gave the green-light on your PR. >>>> 2. You can hope that committers will see your PR, but sometimes things >>> get >>>> lost. If you know someone who is familiar with that area of the code, it >>> is >>>> a good idea to ping them. >>>> 3. We do have some flaky tests. You can see that Jenkins will run 3 >>>> parallel builds, if some of them pass and the committer confirms that >>>> failures are not related to your code, we are ok to merge. Obviously, if >>>> you end up tracking them down and fixing, everyone will be very grateful. >>>> >>>> Hope this helps, >>>> >>>> Gwen >>>> >>>> On Sun, Jun 14, 2020 at 5:52 PM Michael Carter < >>>> michael.car...@instaclustr.com> wrote: >>>> >>>>> Hi all, >>>>> >>>>> I’ve submitted a patch for the first time( >>>>> https://github.com/apache/kafka/pull/8844 < >>>>> https://github.com/apache/kafka/pull/8844>), and I have a couple of >>>>> questions that I’m hoping someone can help me answer. >>>>> >>>>> I’m a little unclear what happens after that patch has been submitted. >>> The >>>>> coding guidelines say Jenkins will run tests automatically, but I don’t >>> see >>>>> any results anywhere. Have I misunderstood what should happen, or do I >>> just >>>>> not know where to look? >>>>> Should I be attempting to find reviewers for the change myself, or is >>> that >>>>> done independently of the patch submitter? >>>>> >>>>> Also, in resolving a couple of conflicts that have arisen after the >>> patch >>>>> was first submitted, I noticed that there are now failing unit tests >>> that >>>>> have nothing to do with my change. Is there a convention on how to deal >>>>> with these? Should it be something that I try to fix on my branch? >>>>> >>>>> Any thoughts are appreciated. >>>>> >>>>> Thanks, >>>>> Michael >>>> >>>> >>>> >>>> -- >>>> Gwen Shapira >>>> Engineering Manager | Confluent >>>> 650.450.2760 | @gwenshap >>>> Follow us: Twitter | blog >>> >>> >
