Hi Luke, The guide is a bit outdated. Thank you for pointing it out. I updated the guide.
As Gwen stated above: > Unfortunately, you need to get a committer to approve running the tests. So, yes a committer has to comment on the PR. Best, Bruno On Fri, Jun 19, 2020 at 1:28 AM Michael Carter <michael.car...@instaclustr.com> wrote: > > 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 > >>> > >>> > > >
