Skip to content

Add characterization tests for SyncRepl (SyncRepl 18% -> 99% line coverage) - #119

Open
opravil-jan wants to merge 2 commits into
apache:masterfrom
opravil-jan:test/syncrepl-characterization
Open

opravil-jan wants to merge 2 commits into
apache:masterfrom
opravil-jan:test/syncrepl-characterization

Conversation

@opravil-jan

Copy link
Copy Markdown

Depends on #116. Without #116, surefire discovers none of the JUnit 5 tests. Until #116 is merged, its commit also shows up here. Only the test(syncrepl) commit belongs to this PR. The branch is independent of #117 and #118.

This PR changes tests only; production code is untouched. It pins down the current behaviour of plugins/openldap.syncrepl, so that later fixes cannot silently change anything else.

What is covered

There are 4 new test classes with 63 tests:

  • Getters, setters and copy() for every option.
  • toString() for every option.
  • Parsing of all 35 keywords and their enum values, including invalid values.
  • Round trips, per option and with all options set together.
lines branches
SyncRepl 18.2% → 98.8% 3.3% → 95.7%
SyncReplParser 55.3% → 95.6% 54.7% → 74.2%
openldap.syncrepl 45.1% → 91.6% 38.2% → 81.9%

Coverage was measured with JaCoCo 0.8.15. Each new class was mutation-checked: I broke a targeted production line and confirmed that a test fails.

Not cementing divergences from slapd

Exact output strings are asserted only where Studio already writes what slapd itself writes, as checked in OpenLDAP master (syncrepl_unparse() in syncrepl.c, bindconf_unparse() and the bindkey table in config.c). For cn=config values, slapd quotes a fixed set of fields and escapes nothing, because strtok_quote_ldif() has no escape mechanism. Where Studio differs, the test asserts the slapd behaviour and is @Disabled. Every disabled test was enabled once and fails for the stated reason.

Suspected bugs (7):

  • exattrs=userPassword is read as attrs=userPassword. Keywords are matched anywhere in the text, because the parser skips one character when nothing matches. The attribute meant to be excluded therefore becomes the only replicated attribute, and that is written back on save.
  • For the same reason, xrid=5 is read as rid=5.
  • Options the parser does not know (e.g. lazycommit, suffixmassage, tls_protocol_min, exattrs) are silently dropped, so they disappear from the value when it is saved.
  • The cipher suite is read and written as tls_ciphersuite=, but slapd only knows tls_cipher_suite=. slapd rejects the value Studio writes, and the cipher suite of an existing consumer is lost. There are 2 tests, one for writing and one for reading.
  • filter= at the very end of the value throws ArrayIndexOutOfBoundsException (SyncReplParser.java:1025) instead of a parser error.
  • The "missing value" error always names option rid, whichever option it is.

Known divergences from slapd (5):

  • credentials and the tls_cert/tls_key/tls_cacert/tls_cacertdir paths are written unquoted.
  • When reading, \" is treated as an escape, a quote that is not followed by whitespace closes the value, and '...' is accepted as quoting.

Fixed by #117 (1): tls_cacertdir round trip.

Two cases are deliberately left untested, with a comment in the test class:

  • A " inside a quoted value: no output string lets slapd read it back unchanged.
  • Keyword case: slapd matches keywords with strncasecmp.

Verification

mvn clean install (Maven 3.9.16, JDK 17): openldap.syncrepl ran 140 tests, of which 127 passed and 13 were skipped (the disabled tests above). openldap.config.editor passed all 34 of its tests.

🤖 Generated with Claude Code

opravil-jan and others added 2 commits September 29, 2026 10:14
junit-platform-runner pulled JUnit 4 onto the test classpath, so
surefire auto-detected the JUnit4 provider and discovered none of the
Jupiter tests: every plugin module reported "Tests run: 0" and the
build stayed green. Nothing uses the runner, so drop it together with
the now unused junit.platform.version property.

Hamcrest was only reachable transitively through JUnit 4; its single
user, EntryEditorWidgetSorterTest, now asserts with JUnit's
assertEquals like the other test classes.

Surefire now runs 360 unit tests across 10 plugins (2 skipped).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pin down the current behaviour of every SyncRepl option: getters and
setters, copy(), toString(), parsing of all 35 keywords and their enum
values, invalid values, and round trips, so that upcoming fixes cannot
silently change anything else. Line coverage of SyncRepl goes from 18%
to 99% and of SyncReplParser from 55% to 96%.

Exact output strings are only asserted where Studio already writes
what slapd's syncrepl_unparse()/bindconf_unparse() write. Differences
from slapd and suspected bugs are @disabled tests that assert the
correct behaviour, to be enabled together with the fix. Among them:
exattrs= is read as attrs=, unknown options are dropped on save, and
the cipher suite uses tls_ciphersuite= where slapd only knows
tls_cipher_suite=.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant