pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

Published 20 hours ago • verified publisher icon apache

[test] Integration test coverage

Open codelipenghui opened this issue 2 years ago • 2 comments

Motivation

Improve integration test coverage https://github.com/apache/pulsar/tree/master/tests/integration The integration test will run on a real cluster, not a mock environment like the unit test does.

Tools

  • [ ] Integration test for Admin CII
  • [ ] Integration test for client tool https://github.com/apache/pulsar/pull/17616
  • [ ] Integration test for perf tool

Security

  • [ ] Integration test for TLS https://github.com/apache/pulsar/pull/17141

Websocket service

  • [ ] With broker
  • [ ] With proxy
  • [ ] Separated service

Plugins

  • [x] Integration test for Protocol handlers #17350 @andrasbeni
  • [ ] Integration test for Broker interceptors #17515
  • [x] Integration test for Broker message filters #17396
  • [x] Integration test for Additional servlet (broker/proxy) #17450

codelipenghui avatar Aug 17 '22 04:08 codelipenghui

@nodece Is handling the integration test of security

codelipenghui avatar Aug 22 '22 04:08 codelipenghui

@codelipenghui , can you please let me know what tests you would like to see for the admin CLI? As I can see, runAdminCommandOnAnyBroker does use the admin cli and is used extensively by integrations tests. And there are some more usages through the constant ADMIN_SCRIPT. So it's not a dedicated test, but I think it already has some coverage.

andrasbeni avatar Sep 20 '22 09:09 andrasbeni

@andrasbeni Most of the admin CLI integration tests are under tests/integration/cli and yes we already have some coverage for this part. I think we should also consider the improve the coverage for this part to avoid someone changing the output format or introducing new incompatible options. But it's not a very strong opinion, we need to maintain so many test codes in the future. WDYT?

codelipenghui avatar Sep 28 '22 13:09 codelipenghui

@codelipenghui , I believe integration tests are too expensive to do a full validation of output and command line parameters. Having taken a quick look at admin command implementations and PulsarAdminToolTest, it looks to me that admin commands are designed to be easily unit tested: the PulsarAdmin can be mocked by setting a Supplier and CaptureStdOut makes their output verifiable. So, in my opinion, we would be much better off writing unit tests for all these.

andrasbeni avatar Sep 29 '22 08:09 andrasbeni

@andrasbeni Sound good to me.

codelipenghui avatar Sep 29 '22 09:09 codelipenghui