Skip to content

Commit c7044f8

Browse files
committed
Make the closure dedup and feature sort tests discriminating
Both behaviours survived a mutation of the implementation. Add a closure fixture that repeats a pair and reports a self-pair, and run the feature sort test under a collation that disagrees with byte order - testthat forces LC_COLLATE=C, where the two sorts are indistinguishable.
1 parent 5352ffb commit c7044f8

5 files changed

Lines changed: 99 additions & 4 deletions

File tree

r/DESCRIPTION

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ Suggests:
2828
jsonlite,
2929
recipes,
3030
testthat (>= 3.0.0),
31-
tibble
31+
tibble,
32+
withr
3233
Config/testthat/edition: 3
3334
Encoding: UTF-8
3435
Config/roxygen2/version: 8.1.0

r/tests/testthat/fixtures/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ it contributes no feature - only its subproperties do.
9393
| `closure-update-batch-1.json` | New pairs from adding concepts 1-4 only (6 pairs), for use with the two-page expansion. |
9494
| `closure-update-batch-2.json` | New pairs from then adding concepts 5-6 (9 pairs). Batches 1 and 2 together give the same 15 pairs as `closure-update.json`. |
9595
| `closure-update-no-group.json` | An update response with no `group` at all, which must contribute zero pairs. |
96+
| `closure-update-duplicates.json` | Reports the pair `(399981008, 404684003)` three times - twice in one group, once in a second - and reports `363346000` as subsuming itself, colliding with the identity cell. The encoding must stay multi-hot, so both cells must be exactly 1. `Matrix::sparseMatrix()` sums duplicate triplets, so this is what catches a missing deduplication step. |
9697

9798
### Pair orientation
9899

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
{
2+
"resourceType": "ConceptMap",
3+
"version": "2",
4+
"name": "6c1e9a0b3f5d47a2b8e0c4d19f7a2b3c",
5+
"status": "active",
6+
"experimental": false,
7+
"date": "2026-08-15T20:29:01+10:00",
8+
"group": [
9+
{
10+
"source": "http://snomed.info/sct",
11+
"sourceVersion": "http://snomed.info/sct/32506021000036107/version/20260731",
12+
"target": "http://snomed.info/sct",
13+
"targetVersion": "http://snomed.info/sct/32506021000036107/version/20260731",
14+
"element": [
15+
{
16+
"code": "399981008",
17+
"target": [
18+
{
19+
"code": "404684003",
20+
"equivalence": "subsumes"
21+
},
22+
{
23+
"code": "404684003",
24+
"equivalence": "subsumes"
25+
}
26+
]
27+
},
28+
{
29+
"code": "363346000",
30+
"target": [
31+
{
32+
"code": "363346000",
33+
"equivalence": "subsumes"
34+
}
35+
]
36+
}
37+
]
38+
},
39+
{
40+
"source": "http://snomed.info/sct",
41+
"sourceVersion": "http://snomed.info/sct/32506021000036107/version/20260731",
42+
"target": "http://snomed.info/sct",
43+
"targetVersion": "http://snomed.info/sct/32506021000036107/version/20260731",
44+
"element": [
45+
{
46+
"code": "399981008",
47+
"target": [
48+
{
49+
"code": "404684003",
50+
"equivalence": "subsumes"
51+
}
52+
]
53+
}
54+
]
55+
}
56+
]
57+
}

r/tests/testthat/test-encoder.R

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,29 @@ test_that("construction produces the documented identity plus subsumption matrix
7070
expect_equal(as.matrix(encoder$encoded), expected_encoding())
7171
})
7272

73+
test_that("a repeated closure pair still sets its cell to 1, not a count", {
74+
# Arrange - a closure response that reports the same (399981008, 404684003)
75+
# pair three times, twice within one group and once in a second group, and
76+
# reports 363346000 as subsuming itself, which collides with the identity
77+
# cell. Servers are free to return either; the encoding must stay multi-hot.
78+
encoder <- suppressMessages(with_fixture_api(
79+
fhir_tx_encoder(scope = test_scope, tx_url = test_tx_url),
80+
expand = "expand-single-page",
81+
closure_update = "closure-update-duplicates"
82+
))
83+
84+
# Assert - the repeated pair and the self-pair are both exactly 1. Building
85+
# the matrix from triplets without deduplicating them would sum the
86+
# duplicates, giving 3 and 2 respectively.
87+
cell <- function(source, target) {
88+
encoder$encoded[encoder$index[[source]], encoder$index[[target]]]
89+
}
90+
expect_equal(cell("399981008", "404684003"), 1)
91+
expect_equal(cell("363346000", "363346000"), 1)
92+
# Nothing anywhere in the encoding may exceed 1.
93+
expect_equal(max(encoder$encoded), 1)
94+
})
95+
7396
test_that("construction records codes, displays, feature names, index and scope", {
7497
# Arrange / Act
7598
encoder <- build_test_encoder()

r/tests/testthat/test-properties.R

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,22 @@ test_that("no properties at all yields a matrix with one row per concept and no
393393
})
394394

395395
test_that("feature names are sorted in byte order rather than the user's locale", {
396-
# Arrange - under the en_AU (and en_US) collation R sorts these as
397-
# "_x=1", "a=1", "Z=1"; scikit-learn's DictVectorizer sorts by code point,
398-
# giving "Z=1", "_x=1", "a=1". A locale-collated sort therefore fails here.
396+
# Arrange - testthat runs tests under LC_COLLATE=C, where a locale-collated
397+
# sort and a byte-order sort agree, so the test would pass either way. Switch
398+
# to a collation that disagrees: under en_AU, R sorts these as "_x=1", "a=1",
399+
# "Z=1", whereas scikit-learn's DictVectorizer sorts by code point, giving
400+
# "Z=1", "_x=1", "a=1".
401+
skip_if_not(
402+
identical(
403+
suppressWarnings(withr::with_locale(
404+
c(LC_COLLATE = "en_AU.UTF-8"),
405+
sort(c("Z", "_x", "a"))
406+
)),
407+
c("_x", "a", "Z")
408+
),
409+
"en_AU.UTF-8 collation is unavailable"
410+
)
411+
withr::local_locale(c(LC_COLLATE = "en_AU.UTF-8"))
399412
lists <- list(list(Z = "1", "_x" = "1", a = "1"))
400413

401414
# Act

0 commit comments

Comments
 (0)