CFSet: add a test suite for creation, lookup and mutation#71
CFSet: add a test suite for creation, lookup and mutation#71DTW-Thalion wants to merge 1 commit into
Conversation
HendrikHuebner
left a comment
HendrikHuebner
left a comment
There was a problem hiding this comment.
Is the implementation of the mutable set generally the same as the immutable set for read only functions?
| CFRelease (set); | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Maybe also add a brief test for empty sets.
|
|
||
| values[0] = CFSTR("a"); | ||
| values[1] = CFSTR("b"); | ||
| values[2] = CFSTR("c"); |
There was a problem hiding this comment.
Can we also have a test for sets created from an array of values with duplicates / NULLs?
There was a problem hiding this comment.
Adding that test surfaced a bug, so I've left it out of this suite rather than assert the broken behaviour: CFSetCreate doesn't de-duplicate. From {a, b, a} it returns count 3, and CFSetGetValues then hands back a garbage third pointer (segfault). Root cause is GSHashTableCreate incrementing _count per array element with no existing-key check, so CFDictionaryCreate/CFBagCreate are affected too. I'll fix that in a separate PR and add the de-dup test (and the NULL-callbacks case) there.
| copy = CFSetCreateMutableCopy (NULL, 0, set); | ||
| PASS_CF(copy != NULL, "CFSetCreateMutableCopy returns a set."); | ||
| PASS_CFEQ(copy, set, "A mutable copy is equal to the original."); | ||
| CFRelease (copy); |
There was a problem hiding this comment.
Also test whether removing an absent value is a no op.
CFSet.c had only partial coverage. Add tests exercising the public API: CFSetCreate, the count and lookup accessors (CFSetGetCount, CFSetContainsValue, CFSetGetCountOfValue, CFSetGetValue, CFSetGetValueIfPresent, CFSetGetValues), CFSetApplyFunction, CFSetCreateCopy, the type ID, and CFEqual / CFHash. The mutable path covers CFSetCreateMutable, CFSetAddValue (including that a present value is not duplicated), CFSetSetValue, CFSetReplaceValue, CFSetRemoveValue, CFSetRemoveAllValues and CFSetCreateMutableCopy.
DTW-Thalion
force-pushed
the
test/cfset-coverage
branch
from
2bb9db4 to
10cdb9f
Compare
|
@HendrikHuebner Yes — immutable and mutable CFSets are both backed by the same GSHashTable, and the read-only functions ( |
2 participants
CFSet.chad only partial test coverage. This adds a self-contained CFSet test suite (Tests/CFSet) built on the existingCFTesting.hframework.Immutable (
create.m):CFSetCreate,CFSetGetCount.CFSetContainsValue,CFSetGetCountOfValue,CFSetGetValue,CFSetGetValueIfPresent,CFSetGetValues.CFSetApplyFunction(verifies every value is visited once).CFSetCreateCopy,CFSetGetTypeID/CFGetTypeID, andCFEqual/CFHash.Mutable (
mutable.m):CFSetCreateMutable,CFSetAddValue(including that a value already present is not duplicated),CFSetSetValue,CFSetReplaceValue(present and absent),CFSetRemoveValue,CFSetRemoveAllValues,CFSetCreateMutableCopy.This raises
CFSet.cline coverage from 40.0% to 89.5% (measured with llvm-cov). All new tests pass and no existing tests are affected. The remaining uncovered lines are the toll-free-bridge (CF_IS_OBJC) paths and the internal formatting-description helper.