Enhance simple test #3675
Enhance simple test #3675
Conversation
|
|
||
| try (ReadHandle rh = result(bk.newOpenLedgerOp().withLedgerId(wh.getId()).withDigestType(DigestType.CRC32C) | ||
| .withPassword(new byte[0]).execute())) { | ||
| rh.read(0, flags.numEntries); |
There was a problem hiding this comment.
Would you please assert the result of rh.read()?
There was a problem hiding this comment.
Fine, the write data is an empty array. I will change the written data and verify it.
|
rerun failure checks |
hangc0276
requested review from
dlg99,
eolivelli,
hezhangjian,
StevenLuMT and
zymap
| @@ -61,8 +65,6 @@ public static class Flags extends CliFlags { | |||
| private int ackQuorumSize = 2; | |||
| @Parameter(names = { "-n", "--num-entries" }, description = "Entries to write (default 100)") | |||
| private int numEntries = 100; | |||
| @Parameter(names = { "-c", "--clean-up" }, description = "Clean up ledger created after simple test") | |||
There was a problem hiding this comment.
It is an easily way to generate a ledger. I am not sure if it should be deleted automatically. If we delete it automatically, the only way to get a ledger is writing a program, it looks very inconvenient.
There was a problem hiding this comment.
This command is for testing the bookkeeper to create a ledger and read the ledger. If already cover the user's case.
If users want to test by themselves, they should create a new one and read it, not support the ledger generated by SimpleTestCommand.
| @@ -81,6 +83,7 @@ public SimpleTestCommand(Flags flags) { | |||
| @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") | |||
| protected void run(BookKeeper bk, Flags flags) throws Exception { | |||
| byte[] data = new byte[100]; // test data | |||
| Arrays.fill(data, (byte) '1'); | |||
There was a problem hiding this comment.
Fill them randomly like how we did in pulsar-perf?
| .withPassword(new byte[0]).execute())) { | ||
| LedgerEntries ledgerEntries = rh.read(0, flags.numEntries); | ||
| for (LedgerEntry ledgerEntry : ledgerEntries) { | ||
| assert Arrays.equals(ledgerEntry.getEntryBytes(), data); |
There was a problem hiding this comment.
The assert is more likes a developer tool not for users. We need to enable it with -ea when executing the program. Does it make more sense to log errors or throw exceptions?
| } | ||
| result(bk.newDeleteLedgerOp().withLedgerId(wh.getId()).execute()); |
### Motivation Fixes apache#3670
|
I'm fixing the integration tests in #4197. It seems that simpletest will always fail and has never been working. There are at least 2 problems:
Fixing in lhotari@6ffda5cb since I cannot get integration tests to pass without it. |
### Motivation Fixes apache#3670
Motivation
Fixes #3670