Conversation
|
Demonstration of new fast feedback: https://builds.apache.org/job/kafka-pr-jdk11-scala2.12/1292/console. The build only ran for 8 minutes instead of 2 hours to report a checkstyle error. |
This reverts commit 3e2d483.
|
What problem are we trying to solve, people don't run checkstyle before submitting their PRs? |
|
@ijuma more or less, yea. I think faster feedback into PRs is generally a useful thing. If you have a silly mistake that trips the checkstyle/findbugs/rat, its better to see that sooner than later. |
|
I'm just trying to understand the use case as there may be things we can do to improve the feedback cycle even further. In a sense, if you have to wait for a PR to find out about a checkstyle failure, you sort of lost already. It wastes the time of the reviewer and it's definitely slower than if you had seen it before submitting the change. And running it locally is very fast. With regards to the --continue flag, I added that because people would often fix one test and then there would be more to fix. It made the review cycle really slow. And since the tests take so long to run, it's harder to encourage people to actually run the whole test suite locally before submitting the changes. Another option would be to have multiple jobs, one for validation and one for tests. |
|
I agree developers should run the unit tests and validation checks at a minimum, but that probably isn't always going to happen. Unless we add git hooks requiring certain gradle tasks before sync'ing, it will occasionally get missed. The point of this check/test separation is really just to cover that case and get fast feedback to the developer. |
|
|
||
| # Run tests | ||
| ./gradlew unitTest integrationTest \ | ||
| --profile --no-daemon --continue -PtestLoggingEvents=started,passed,skipped,failed "$@" \ |
There was a problem hiding this comment.
It seems like the --profile information is printed to a file. How would one see that for a Jenkins build?
There was a problem hiding this comment.
@mumrah Can you elaborate on this one? Apart from that, I think we are good to merge.
There was a problem hiding this comment.
Sure: the --profile flag tells gradle to time all of the tasks in during a build. It breaks it down so you can see how much time is spent during the different task phases. It's quite useful when investigating slow builds. The HTML reports generated by the profiling could be archived by Jenkins and/or published using the HTML publisher plugin (similar to a coverage report or junit results)
There was a problem hiding this comment.
Can you please file a JIRA with details about what we should do in Jenkins to make this useful?
|
There are a couple of ways we could improve this for developers:
In any case, we don't have to block on those. This PR seems fine, I just had a question. |
* AK/trunk: fix typo (apache#5150) MINOR: Reduce replica.fetch.backoff.ms in ReassignPartitionsClusterTest (apache#5887) KAFKA-7766: Fail fast PR builds (apache#6059) KAFKA-7798: Expose embedded clientIds (apache#6107) KAFKA-7641; Introduce "group.max.size" config to limit group sizes (apache#6163) KAFKA-7433; Introduce broker options in TopicCommand to use AdminClient (KIP-377) MINOR: Fix some field definitions for ListOffsetReponse (apache#6214) KAFKA-7873; Always seek to beginning in KafkaBasedLog (apache#6203) KAFKA-7719: Improve fairness in SocketServer processors (KIP-402) (apache#6022) MINOR: fix checkstyle suppressions for generated RPC code to work on Windows KAFKA-7859: Use automatic RPC generation in LeaveGroups (apache#6188) KAFKA-7652: Part II; Add single-point query for SessionStore and use for flushing / getter (apache#6161) KAFKA-3522: Add RocksDBTimestampedStore (apache#6149) KAFKA-3522: Replace RecordConverter with TimestampedBytesStore (apache#6204)
Split the Gradle invocation in the jenkins.sh script into two commands so we can fail fast for validation checks such as compile errors and checkstyle errors. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Ismael Juma <ismael@juma.me.uk>
3 participants
Split the Gradle invocation in the jenkins.sh script into two commands so we can fail fast for validation checks such as compile errors and checkstyle errors.
Committer Checklist (excluded from commit message)