Skip to content

MINOR: Reduce build time by gating test coverage plugins behind a flag#8899

Merged
ijuma merged 2 commits intoapache:trunkfrom
ijuma:conditional-coverage
Jun 19, 2020
Merged

MINOR: Reduce build time by gating test coverage plugins behind a flag#8899
ijuma merged 2 commits intoapache:trunkfrom
ijuma:conditional-coverage

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Jun 18, 2020

Most builds don't require test coverage output, so it's wasteful
to spend cycles tracking coverage information for each method
invoked.

I ran a quick test in a fast desktop machine, the absolute
difference will be larger in a slower machine. The tests were
executed after ./gradlew clean and with a gradle daemon
that was started just before the test (and mildly warmed up
by running ./gradlew clean again).

./gradlew unitTest --continue --profile:

  • With coverage enabled: 6m32s
  • With coverage disabled: 5m47s

I ran the same test twice and the results were within 2s of
each other, so reasonably consistent.

16% reduction in the time taken to run the unit tests is a
reasonable gain with little downside, so I think this is a
good change.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Most builds don't require test coverage output, so it's wasteful
to spend cycles tracking coverage information for each method
invoked.

I ran a quick test in a fast desktop machine, the absolute
difference will be larger in a slower machine. The tests were
executed after `./gradlew clean` and with a gradlew daemon
that was started just before the test (and mildly warmed up
with `./gradlew clean` again).

`./gradlew unitTest --continue --profile`:
* With coverage enabled: 6m32s
* With coverage disabled: 5m47s

I ran the same test twice and the results were within 2s of
each other, so reasonably consistent.

16% reduction in the time taken to run the unit tests is a
reasonable gain with little downside, so I think this is a
good change.
@ijuma ijuma changed the title MINOR: Gate test coverage plugin behind Gradle property MINOR: Reduce build time by gating test coverage plugins behind a flag Jun 18, 2020
@ijuma ijuma requested a review from omkreddy June 18, 2020 23:03
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jun 19, 2020

Flaky test failure:

org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication

Copy link
Copy Markdown
Contributor

@omkreddy omkreddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijuma Thanks for the PR. LGTM.

@ijuma ijuma merged commit 5ecd094 into apache:trunk Jun 19, 2020
@ijuma ijuma deleted the conditional-coverage branch June 19, 2020 14:32
Kvicii pushed a commit to Kvicii/kafka that referenced this pull request Jun 21, 2020
* 'trunk' of github.com:apache/kafka:
  KAFKA-10168: fix StreamsConfig parameter name variable (apache#8865)
  MINOR: code cleanup for inconsistent naming (apache#8871)
  KAFKA-10138: Prefer --bootstrap-server for reassign_partitions command in ducktape tests (apache#8898)
  KAFKA-10185: Restoration info logging (apache#8896)
  KAFKA-9891: add integration tests for EOS and StandbyTask (apache#8890)
  MINOR: Reduce build time by gating test coverage plugins behind a flag (apache#8899)
  KAFKA-10141; Add more detail to log segment delete messages (apache#8850)
  KAFKA-10113; Specify fetch offsets correctly in `LogTruncationException` (apache#8822)
  KAFKA-10167: use the admin client to read end-offset (apache#8876)
  MINOR: Upgrade ducktape to 0.7.8 (apache#8879)
  KAFKA-10123; Fix incorrect value for AWAIT_RESET#hasPosition (apache#8841)
  KAFKA-9896: fix flaky StandbyTaskEOSIntegrationTest (apache#8883)
  MINOR: clean up unused checkstyle suppressions for Streams (apache#8861)
  MINOR: reuse toConfigObject(Map) to generate Config (apache#8889)
  MINOR: Upgrade jetty to 9.4.27.v20200227 and jersey to 2.31 (apache#8859)
  MINOR: Fix flaky HighAvailabilityTaskAssignorIntegrationTest (apache#8884)
  KAFKA-10147 MockAdminClient#describeConfigs(Collection<ConfigResource>) is unable to handle broker resource (apache#8853)
  KAFKA-10165: Remove Percentiles from e2e metrics (apache#8882)

# Conflicts:
#	core/src/main/scala/kafka/log/Log.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants