From 14a0fe247daed1fb4d9d84e9be2f6e3a303ddf4e Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 12:08:09 +0200 Subject: [PATCH 01/19] Added regression test for click-modifier, updated pixi.toml with tasks and add-features --- pixi.toml | 5 ++ tests/test-regression-click-modifier.R | 81 ++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 tests/test-regression-click-modifier.R diff --git a/pixi.toml b/pixi.toml index da4de45..c4a149a 100644 --- a/pixi.toml +++ b/pixi.toml @@ -6,6 +6,7 @@ platforms = ["linux-64"] version = "0.1.0" [tasks] +test = [{ task = "tests"}] [dependencies] r = "4.*" @@ -16,6 +17,7 @@ r-stringi = ">=1.8.7,<2" [feature.agg.tasks] aggregate = { cmd = "Rscript aggregate-raw-data.R", cwd = "scripts" } +add_features = { cmd = "Rscript add-features.R", cwd = "scripts" } [feature.ci.dependencies] r-dplyr = "1.1.1.*" @@ -26,6 +28,9 @@ r-stringr = ">=1.5.1,<2" r-testthat = ">=3.2.1.1,<4" r-withr = ">=3.0.0,<4" +[feature.ci.tasks] +tests = { cmd = "Rscript -e \"testthat::test_dir('.', stop_on_failure=TRUE)\"", cwd = "tests" } + [environments] agg = { features = ["agg"] } ci = { features = ["ci"] } diff --git a/tests/test-regression-click-modifier.R b/tests/test-regression-click-modifier.R new file mode 100644 index 0000000..2286929 --- /dev/null +++ b/tests/test-regression-click-modifier.R @@ -0,0 +1,81 @@ +#! /usr/bin/env Rscript + +## Regression test for the click/modifier typing in order_ipa() (aggregation-helper-functions.R). +## Clicks are deliberately typed "M", the same glyph-type code as real modifier letters +## so they factor into feature-vector assignment the same way +## (see create_glyph_type_variables()/make_typestring()). + +library(stringi, warn.conflicts=FALSE) +library(testthat, warn.conflicts=FALSE) + +source(file.path("..", "scripts", "aggregation-helper-functions.R")) + +context("Click/modifier regression") + +test_that("a click is not deleted when followed by a modifier letter", { + ## dental click U+01C0 + aspirated U+02B0 both typed "M" + ## triggering the "MM" canonical-reordering block + result <- order_ipa("ǀʰ") + expect(stri_detect_fixed(result, "ǀ"), + paste("Click character was lost from", shQuote(result))) + expect(nchar(result) == 2, + paste("Expected a 2-character result, got", shQuote(result), + "(", nchar(result), "characters)")) +}) + +test_that("a click is not deleted when preceding a modifier letter", { + ## same "MM" span, reversed input order + result <- order_ipa("ʰǀ") + expect(stri_detect_fixed(result, "ǀ"), + paste("Click character was lost from", shQuote(result))) + expect(nchar(result) == 2, + paste("Expected a 2-character result, got", shQuote(result), + "(", nchar(result), "characters)")) +}) + +test_that("a click followed by a diacritic is not swapped the way a modifier would be", { + ## dental click + devoiced/ring-below diacritic (U+0325) - Typed "MD" + ## Should not be swapped to "DM" because the click is not a real modifier letter. + result <- order_ipa("ǀ̥") + expect(stri_detect_fixed(result, "ǀ"), + paste("Click character was lost from", shQuote(result))) + expect(identical(stri_sub(result, 1, 1), "ǀ"), + paste("Expected click to remain first, got", shQuote(result))) +}) + +test_that("multiple clicks in sequence with a modifier are all preserved", { + ## two different clicks + one modifier, all typed "M". + result <- order_ipa("ǀǁʰ") + expect(stri_detect_fixed(result, "ǀ"), + paste("First click character was lost from", shQuote(result))) + expect(stri_detect_fixed(result, "ǁ"), + paste("Second click character was lost from", shQuote(result))) + expect(nchar(result) == 3, + paste("Expected a 3-character result, got", shQuote(result), + "(", nchar(result), "characters)")) +}) + +test_that("two separate click+diacritic pairs in one string are both skipped", { + ## typestring "MDMD" both with clicks, so both should be skipped + result <- order_ipa("ǀ̥ǁ̥") + expect(stri_detect_fixed(result, "ǀ"), + paste("First click character was lost from", shQuote(result))) + expect(stri_detect_fixed(result, "ǁ"), + paste("Second click character was lost from", shQuote(result))) + expect(nchar(result) == 4, + paste("Expected a 4-character result, got", shQuote(result), + "(", nchar(result), "characters)")) +}) + +test_that("a click preceding a real modifier+diacritic doesn't block their swap", { + ## typestring "MMD" with a click first, then a modifier letter, then a diacritic. + ## The modifier+diacritic pair should still be swapped, but the click should remain first. + result <- order_ipa("ǀʰ̥") + expect(stri_detect_fixed(result, "ǀ"), + paste("Click character was lost from", shQuote(result))) + expect(identical(stri_sub(result, 1, 1), "ǀ"), + paste("Expected click to remain first, got", shQuote(result))) + expect(identical(result, "ǀ̥ʰ"), + paste("Expected the modifier+diacritic pair to still be swapped", + "(click skip should not suppress it), got", shQuote(result))) +}) From 66016df804722c34bdf87d15013f22e521b781be Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 12:34:17 +0200 Subject: [PATCH 02/19] Added regression test for special-cases and c-cedilla --- tests/test-regression-special-casess.R | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/test-regression-special-casess.R diff --git a/tests/test-regression-special-casess.R b/tests/test-regression-special-casess.R new file mode 100644 index 0000000..75f5aad --- /dev/null +++ b/tests/test-regression-special-casess.R @@ -0,0 +1,33 @@ +#! /usr/bin/env Rscript + +## Regression test for two special-cased exceptions in the R pipeline: +## c-cedilla staying precomposed in order_ipa() (aggregation-helper-functions.R) and +## the `special_cases` affricate list in add-features.R having a +## matching row in special-feature-table.csv for each pair. + +library(stringi, warn.conflicts=FALSE) +library(testthat, warn.conflicts=FALSE) + +source(file.path("..", "scripts", "aggregation-helper-functions.R")) + +context("Special-case exceptions regression") + +test_that("known special-cased exceptions in the pipeline still work", { + ## order_ipa() leaves c-cedilla unchanged, rather than decomposing it + ordered <- order_ipa("ç") + expect(identical(ordered, "ç"), + paste("Expected c-cedilla phoneme to be unchanged by order_ipa(), got", + shQuote(ordered))) + + ## every add-features.R `special_cases` pair has a matching row in special-feature-table.csv + special_cases <- c("pɸ", "pf", "tθ", "ts", "tʃ", "ʈʂ", "cç", "kx", + "qχ", "bβ", "bv", "dð", "dz", "dʒ", "ɖʐ", "ɟʝ", "ɡɣ", + "ɢʁ", "kp", "ɡb") + special_feats <- read.csv(file.path("..", "raw-data", "FEATURES", + "special-feature-table.csv"), + fileEncoding="UTF-8") + missing <- setdiff(special_cases, special_feats$segment) + expect(length(missing) == 0, + paste("special_cases pair(s) missing from special-feature-table.csv:", + paste(missing, collapse=", "))) +}) From 776e54843d0ea63d813c5aebdab5c4574a2bb832 Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 13:30:41 +0200 Subject: [PATCH 03/19] Excluded test-phonemes.R from ci, added output as pr comment --- .github/workflows/main.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a1729ea..fc714d7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [ master ] +permissions: + pull-requests: write + jobs: test-data: runs-on: ubuntu-latest @@ -18,4 +21,14 @@ jobs: - name: run tests working-directory: tests run: | - Rscript -e "testthat::test_dir('.', stop_on_failure=TRUE)" + Rscript -e "testthat::test_dir('.', filter = \"^(?test-phoenemes)\", stop_on_failure = TRUE)" + - name: run feature test (write to PR comment) + working-directory: tests + run: | + Rscript -e "testthat::test_file('test-phonemes.R', reporter = \"summary\")" > phoneme_feature_test_results.txt 2>&1 || true + - name: comment on PR + if: github.event_name == 'pull_request' + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt From 21b0eb50462147ebd6bda0d06b098ecf19a2c2b4 Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 13:46:20 +0200 Subject: [PATCH 04/19] Fixed typo and test commands in workflows --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fc714d7..244a7f3 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -21,11 +21,11 @@ jobs: - name: run tests working-directory: tests run: | - Rscript -e "testthat::test_dir('.', filter = \"^(?test-phoenemes)\", stop_on_failure = TRUE)" + Rscript -e "testthat::test_dir('.', filter = 'phonemes', invert = TRUE, stop_on_failure = TRUE)" - name: run feature test (write to PR comment) working-directory: tests run: | - Rscript -e "testthat::test_file('test-phonemes.R', reporter = \"summary\")" > phoneme_feature_test_results.txt 2>&1 || true + Rscript -e "testthat::test_file('test-phonemes.R', reporter = 'summary')" > phoneme_feature_test_results.txt 2>&1 || true - name: comment on PR if: github.event_name == 'pull_request' env: From d8d96b9c54289d4f5838f8da659e723f39beb7eb Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 13:49:22 +0200 Subject: [PATCH 05/19] changed working directory of pr comment job --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 244a7f3..b9c90ae 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -28,6 +28,7 @@ jobs: Rscript -e "testthat::test_file('test-phonemes.R', reporter = 'summary')" > phoneme_feature_test_results.txt 2>&1 || true - name: comment on PR if: github.event_name == 'pull_request' + working-directory: tests env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | From bb31120c9f1851d0bd3171f411f11e22deb3271c Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 13:54:31 +0200 Subject: [PATCH 06/19] Added comment about workflow change --- .github/workflows/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b9c90ae..4148e0f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,6 +22,8 @@ jobs: working-directory: tests run: | Rscript -e "testthat::test_dir('.', filter = 'phonemes', invert = TRUE, stop_on_failure = TRUE)" + ## The following steps are temporary until we fix the phoneme feature test to pass in CI. + ## Once the phoneme feature test passes, update the above command and remove filter and invert, and delete the following steps. - name: run feature test (write to PR comment) working-directory: tests run: | From 6add4c27ea5d443aeb98ebc482dd688e278b2fb2 Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 14:07:21 +0200 Subject: [PATCH 07/19] Added comment to test workflow --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4148e0f..97eb20f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -28,6 +28,7 @@ jobs: working-directory: tests run: | Rscript -e "testthat::test_file('test-phonemes.R', reporter = 'summary')" > phoneme_feature_test_results.txt 2>&1 || true + ## Adds a comment to the PR with the results of the phoneme feature test. - name: comment on PR if: github.event_name == 'pull_request' working-directory: tests From 9d695d8c6cfb6e0668a68cb4f8a88b42b34d9a96 Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 14:18:12 +0200 Subject: [PATCH 08/19] Testing edit-last in gh pr comment --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 97eb20f..10c8cd9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -35,4 +35,4 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt + gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt --edit-last --create-if-none From f45522885ba0154627a16670c7ed190658fc8563 Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 14:20:03 +0200 Subject: [PATCH 09/19] reverted edit-last in gh pr comment --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 10c8cd9..89bce93 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -35,4 +35,4 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt --edit-last --create-if-none + gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt From 7a34af48dac0a62cc2de8c80cebf5769178db2d8 Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 16:30:29 +0200 Subject: [PATCH 10/19] Added test to verify glottocode-isocode mapping matches glottolog --- tests/test-inventory-metadata.R | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test-inventory-metadata.R b/tests/test-inventory-metadata.R index 2aaca1a..754db28 100644 --- a/tests/test-inventory-metadata.R +++ b/tests/test-inventory-metadata.R @@ -141,6 +141,7 @@ test_that("glottocodes are valid", { glotto_url <- "https://github.com/glottolog/glottolog-cldf/blob/master/cldf/languages.csv?raw=true" glotto_table <- readr::read_csv(glotto_url, col_types=default_char_cols) glotto_valid <- pull(glotto_table, ID) + iso_valid_map <- pull(glotto_table, ISO639P3code) glotto_phoible <- pull(phoible, Glottocode) glotto_invalid <- setdiff(glotto_phoible, glotto_valid) @@ -166,6 +167,39 @@ test_that("glottocodes are valid", { collapse=" "), sep="\n") ) + + ## Check that the ISO code in glottolog matches phoible's ISO code, for + ## every inventory with a known Glottocode (i.e. excluding Djindewal). + has_glottocode <- !is.na(phoible$Glottocode) + + iso_from_glottolog <- iso_valid_map[match(phoible$Glottocode[has_glottocode], glotto_valid)] + iso_from_phoible <- phoible$ISO6393[has_glottocode] + + ## Exclude known glottolog-ISO mismatches + known_mismatches <- rbind( + c(868, "nepa1253", "kru"), # kxl (deprecated) merged into kru + c(1307, "krim1238", "bmf"), # krm (deprecated) merged into bmf + c(2750, "dira1238", "dif"), # dit (deprecated) merged into dif + c(2875, "band1337", "drl"), # bjd (deprecated) merged into drl + c(2256, "dark1243", "khk") # drh (deprecated) merged into khk + ) + + overridable <- phoible$Glottocode[has_glottocode] %in% known_mismatches[, 2] + override <- setNames(known_mismatches[, 3], known_mismatches[, 2]) + iso_from_glottolog[overridable] <- override[phoible$Glottocode[has_glottocode][overridable]] + + mismatch_indices <- which(iso_from_glottolog != iso_from_phoible) + + mismatch_ids <- phoible$InventoryID[has_glottocode][mismatch_indices] + mismatch_glottocodes <- phoible$Glottocode[has_glottocode][mismatch_indices] + mismatches <- unique(paste0(mismatch_ids, " (", mismatch_glottocodes, "): ", iso_from_phoible[mismatch_indices], + " - ", iso_from_glottolog[mismatch_indices])) + + expect(length(mismatches) == 0, + paste("MISMATCHED ISO CODES BETWEEN PHOIBLE AND GLOTTOLOG (InventoryID (glottocode)): phoible - glottolog):", + paste0("- ", mismatches, collapse="\n"), + sep="\n") + ) } ) From 1e6d4513b78ef247e8e79a2ac234c6ec2fe8b24a Mon Sep 17 00:00:00 2001 From: varunghat Date: Thu, 20 Aug 2026 16:37:12 +0200 Subject: [PATCH 11/19] Added test to check inventory-bibtex mapping, InventoryID duplicates --- tests/test-cross-inventory-integrity.R | 100 +++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/test-cross-inventory-integrity.R diff --git a/tests/test-cross-inventory-integrity.R b/tests/test-cross-inventory-integrity.R new file mode 100644 index 0000000..3c4f1d7 --- /dev/null +++ b/tests/test-cross-inventory-integrity.R @@ -0,0 +1,100 @@ +#! /usr/bin/env Rscript + +## Tests for cross-inventory integrity. Checks that: +## 1. No source accidentally split one language (Glottocode, LanguageName, SpecificDialect, BibtexKey) across two InventoryIDs +## 2. Every BibtexKey in the mapping file resolves to a real entry in +## phoible-references.bib. +## 3. Every InventoryID in the mapping files exists in the aggregated +## data, and vice versa. + +library(dplyr, warn.conflicts=FALSE) +library(testthat, warn.conflicts=FALSE) + +## load PHOIBLE data +phoible_data_file <- file.path("..", "data", "phoible.csv") +phoible_col_types <- readr::cols(InventoryID="c", Marginal="l", .default="c") +phoible <- readr::read_csv(phoible_data_file, col_types=phoible_col_types) + +bibtex_mapping_file <- file.path("..", "mappings", "InventoryID-Bibtex.csv") +bibtex_mapping <- readr::read_csv(bibtex_mapping_file, col_types=readr::cols(.default="c")) + +# Join the phoible data with the mapping file to get a combined dataset for testing +phoible_with_bibtex <- phoible %>% + left_join(bibtex_mapping, by="InventoryID", relationship="many-to-many") + +references_bib_file <- file.path("..", "mappings", "phoible-references.bib") + +context("Cross-inventory integrity") + +test_that("no source accidentally split one language across two InventoryIDs", { + ## Checks whether the same (Source, Glottocode, LanguageName, SpecificDialect, BibtexKey) tuple + ## appears in more than one InventoryID. + phoible_with_bibtex %>% + filter(!is.na(Glottocode)) %>% + distinct(InventoryID, Source, Glottocode, LanguageName, SpecificDialect, BibtexKey) %>% + group_by(Source, Glottocode, LanguageName, SpecificDialect, BibtexKey) %>% + filter(n_distinct(InventoryID) > 1) %>% + ungroup() -> + split_duplicates + + expect(nrow(split_duplicates) == 0, + paste(c("SAME (Source, Glottocode, LanguageName, SpecificDialect, BibtexKey) APPEARS IN MULTIPLE INVENTORYIDS:", + capture.output(print(split_duplicates, n=Inf))), + sep="\n") + ) + } +) + +test_that("every BibtexKey resolves to a real bib entry", { + ## Check that every BibtexKey in the mapping file resolves to a real entry in phoible-references.bib. + + ## regex to match BibTeX keys in the .bib file + citekey_re <- "(?m)^@\\w+\\s*\\{\\s*([^,]+?)\\s*," + + bib_keys <- stringr::str_match_all( + readr::read_file(references_bib_file), citekey_re + )[[1]][, 2] + + mapping_keys <- bibtex_mapping %>% pull(BibtexKey) %>% unique() + missing_keys <- setdiff(mapping_keys, bib_keys) + + # Exclude special case of InventoryID 201 (Arrente), which has "NO SOURCE GIVEN" as its BibtexKey + missing_keys <- setdiff(missing_keys, bibtex_mapping %>% filter(InventoryID == 201) %>% pull(BibtexKey)) + + ## report which InventoryID(s) cite each missing key + offending_rows <- bibtex_mapping %>% + filter(BibtexKey %in% missing_keys) %>% + arrange(BibtexKey, as.integer(InventoryID)) + offending_summary <- offending_rows %>% + group_by(BibtexKey) %>% + summarise(InventoryIDs = paste(InventoryID, collapse=", "), .groups="drop") + + expect(length(missing_keys) == 0, + paste(c(paste("BIBTEXKEYS WITH NO MATCHING ENTRY IN", + basename(references_bib_file)), + paste0(offending_summary$BibtexKey, " (InventoryID ", + offending_summary$InventoryIDs, ")")), + sep="\n") + ) + } +) + +test_that("mapping-file and aggregated-data InventoryIDs match in both directions", { + phoible_ids <- phoible %>% pull(InventoryID) %>% unique() + mapping_ids <- bibtex_mapping %>% pull(InventoryID) %>% as.integer() %>% unique() + + only_in_mapping <- setdiff(mapping_ids, phoible_ids) + only_in_phoible <- setdiff(phoible_ids, mapping_ids) + + expect(length(only_in_mapping) == 0, + paste(c("INVENTORYIDS IN InventoryID-Bibtex.csv BUT MISSING FROM", + "AGGREGATED DATA:", paste(only_in_mapping, collapse=" ")), + sep="\n") + ) + expect(length(only_in_phoible) == 0, + paste(c("INVENTORYIDS IN AGGREGATED DATA BUT MISSING FROM", + "InventoryID-Bibtex.csv:", paste(only_in_phoible, collapse=" ")), + sep="\n") + ) + } +) From f8d085381c7d49b65901f12e7715c9bb385b0840 Mon Sep 17 00:00:00 2001 From: varunghat Date: Fri, 21 Aug 2026 00:44:14 +0200 Subject: [PATCH 12/19] Moved feature test to beginning so we get pr comment even if other tests fail --- .github/workflows/main.yml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 89bce93..18a318c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,12 +18,8 @@ jobs: with: pixi-version: v0.66.0 activate-environment: "ci" - - name: run tests - working-directory: tests - run: | - Rscript -e "testthat::test_dir('.', filter = 'phonemes', invert = TRUE, stop_on_failure = TRUE)" ## The following steps are temporary until we fix the phoneme feature test to pass in CI. - ## Once the phoneme feature test passes, update the above command and remove filter and invert, and delete the following steps. + ## Once the phoneme feature test passes, update the below run tests command and remove filter and invert, and delete the following steps. - name: run feature test (write to PR comment) working-directory: tests run: | @@ -36,3 +32,9 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt + + - name: run tests + working-directory: tests + run: | + Rscript -e "testthat::test_dir('.', filter = 'phonemes', invert = TRUE, stop_on_failure = TRUE)" + \ No newline at end of file From 9e1a2e4a60a1fe363615a182a5e692d4071fff7d Mon Sep 17 00:00:00 2001 From: varunghat Date: Fri, 21 Aug 2026 01:04:10 +0200 Subject: [PATCH 13/19] Moved workflow permissions to inside jobs --- .github/workflows/main.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 18a318c..3039e5d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -6,12 +6,12 @@ on: pull_request: branches: [ master ] -permissions: - pull-requests: write - jobs: test-data: runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write steps: - uses: actions/checkout@v7 - uses: prefix-dev/setup-pixi@v0.10.1 @@ -32,7 +32,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt - + - name: run tests working-directory: tests run: | From 42b863cb318e367166262752211115f1f6483fce Mon Sep 17 00:00:00 2001 From: varunghat Date: Fri, 21 Aug 2026 01:22:45 +0200 Subject: [PATCH 14/19] Split PR comment workflow to allow for PR from forked repos --- .github/workflows/comment-feature-test.yml | 30 ++++++++++++++++++++++ .github/workflows/main.yml | 24 ++++++++++------- 2 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/comment-feature-test.yml diff --git a/.github/workflows/comment-feature-test.yml b/.github/workflows/comment-feature-test.yml new file mode 100644 index 0000000..d4def20 --- /dev/null +++ b/.github/workflows/comment-feature-test.yml @@ -0,0 +1,30 @@ +name: comment feature test results + +on: + workflow_run: + workflows: ["test data"] + types: [completed] + +permissions: + actions: read + pull-requests: write + +jobs: + comment: + runs-on: ubuntu-latest + if: github.event.workflow_run.event == 'pull_request' + steps: + - name: download feature test results + uses: actions/download-artifact@v4 + with: + name: feature-test-results + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: comment on PR + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + run: | + PR_NUMBER=$(cat pr_number.txt) + gh pr comment "$PR_NUMBER" --body-file phoneme_feature_test_results.txt diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3039e5d..f2962aa 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -5,33 +5,39 @@ on: branches: [ master ] pull_request: branches: [ master ] - + jobs: test-data: runs-on: ubuntu-latest permissions: contents: read - pull-requests: write + actions: write steps: - uses: actions/checkout@v7 - uses: prefix-dev/setup-pixi@v0.10.1 with: pixi-version: v0.66.0 activate-environment: "ci" - ## The following steps are temporary until we fix the phoneme feature test to pass in CI. + ## The following steps are temporary until we fix the phoneme feature test to pass in CI. ## Once the phoneme feature test passes, update the below run tests command and remove filter and invert, and delete the following steps. - name: run feature test (write to PR comment) working-directory: tests run: | Rscript -e "testthat::test_file('test-phonemes.R', reporter = 'summary')" > phoneme_feature_test_results.txt 2>&1 || true - ## Adds a comment to the PR with the results of the phoneme feature test. - - name: comment on PR + ## Fork PRs get a read-only GITHUB_TOKEN here, so this job can't comment + ## directly. comment-feature-test.yml (triggered via workflow_run) does it instead. + - name: save PR number if: github.event_name == 'pull_request' working-directory: tests - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - gh pr comment ${{ github.event.pull_request.number }} --body-file phoneme_feature_test_results.txt + run: echo "${{ github.event.pull_request.number }}" > pr_number.txt + - name: upload feature test results + if: github.event_name == 'pull_request' + uses: actions/upload-artifact@v4 + with: + name: feature-test-results + path: | + tests/phoneme_feature_test_results.txt + tests/pr_number.txt - name: run tests working-directory: tests From 520b59211adbbda89351c2ae076a91adedc4ea28 Mon Sep 17 00:00:00 2001 From: varunghat Date: Sun, 23 Aug 2026 14:15:00 +0200 Subject: [PATCH 15/19] Removed write permissions from main.yml actions, removed stray whitespaces --- .github/workflows/main.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f2962aa..9fec851 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -5,13 +5,11 @@ on: branches: [ master ] pull_request: branches: [ master ] - jobs: test-data: runs-on: ubuntu-latest permissions: contents: read - actions: write steps: - uses: actions/checkout@v7 - uses: prefix-dev/setup-pixi@v0.10.1 @@ -38,9 +36,8 @@ jobs: path: | tests/phoneme_feature_test_results.txt tests/pr_number.txt - + ## Run the rest of the tests, excluding the phoneme feature test (which is run above). - name: run tests working-directory: tests run: | - Rscript -e "testthat::test_dir('.', filter = 'phonemes', invert = TRUE, stop_on_failure = TRUE)" - \ No newline at end of file + Rscript -e "testthat::test_dir('.', filter = 'phonemes', invert = TRUE, stop_on_failure = TRUE)" \ No newline at end of file From 9af55311dba594647b11e4ed98436bd2b0a5b7bc Mon Sep 17 00:00:00 2001 From: varunghat Date: Sun, 23 Aug 2026 14:18:58 +0200 Subject: [PATCH 16/19] Removed two unnecessary tests from click-modifier regression test --- tests/test-regression-click-modifier.R | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/tests/test-regression-click-modifier.R b/tests/test-regression-click-modifier.R index 2286929..1615ec2 100644 --- a/tests/test-regression-click-modifier.R +++ b/tests/test-regression-click-modifier.R @@ -43,30 +43,6 @@ test_that("a click followed by a diacritic is not swapped the way a modifier wou paste("Expected click to remain first, got", shQuote(result))) }) -test_that("multiple clicks in sequence with a modifier are all preserved", { - ## two different clicks + one modifier, all typed "M". - result <- order_ipa("ǀǁʰ") - expect(stri_detect_fixed(result, "ǀ"), - paste("First click character was lost from", shQuote(result))) - expect(stri_detect_fixed(result, "ǁ"), - paste("Second click character was lost from", shQuote(result))) - expect(nchar(result) == 3, - paste("Expected a 3-character result, got", shQuote(result), - "(", nchar(result), "characters)")) -}) - -test_that("two separate click+diacritic pairs in one string are both skipped", { - ## typestring "MDMD" both with clicks, so both should be skipped - result <- order_ipa("ǀ̥ǁ̥") - expect(stri_detect_fixed(result, "ǀ"), - paste("First click character was lost from", shQuote(result))) - expect(stri_detect_fixed(result, "ǁ"), - paste("Second click character was lost from", shQuote(result))) - expect(nchar(result) == 4, - paste("Expected a 4-character result, got", shQuote(result), - "(", nchar(result), "characters)")) -}) - test_that("a click preceding a real modifier+diacritic doesn't block their swap", { ## typestring "MMD" with a click first, then a modifier letter, then a diacritic. ## The modifier+diacritic pair should still be swapped, but the click should remain first. From f1c78ca125a6ea5cc9556e9d15b6745051391213 Mon Sep 17 00:00:00 2001 From: varunghat Date: Sun, 23 Aug 2026 14:34:46 +0200 Subject: [PATCH 17/19] Changed special-case regression test to only c-cedilla --- tests/test-regression-special-casess.R | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/tests/test-regression-special-casess.R b/tests/test-regression-special-casess.R index 75f5aad..785e9f4 100644 --- a/tests/test-regression-special-casess.R +++ b/tests/test-regression-special-casess.R @@ -1,9 +1,7 @@ #! /usr/bin/env Rscript -## Regression test for two special-cased exceptions in the R pipeline: -## c-cedilla staying precomposed in order_ipa() (aggregation-helper-functions.R) and -## the `special_cases` affricate list in add-features.R having a -## matching row in special-feature-table.csv for each pair. +## Regression test for c-cedilla staying precomposed in order_ipa() +## (aggregation-helper-functions.R), rather than decomposing it. library(stringi, warn.conflicts=FALSE) library(testthat, warn.conflicts=FALSE) @@ -12,22 +10,9 @@ source(file.path("..", "scripts", "aggregation-helper-functions.R")) context("Special-case exceptions regression") -test_that("known special-cased exceptions in the pipeline still work", { - ## order_ipa() leaves c-cedilla unchanged, rather than decomposing it +test_that("c-cedilla is unchanged by order_ipa()", { ordered <- order_ipa("ç") expect(identical(ordered, "ç"), paste("Expected c-cedilla phoneme to be unchanged by order_ipa(), got", shQuote(ordered))) - - ## every add-features.R `special_cases` pair has a matching row in special-feature-table.csv - special_cases <- c("pɸ", "pf", "tθ", "ts", "tʃ", "ʈʂ", "cç", "kx", - "qχ", "bβ", "bv", "dð", "dz", "dʒ", "ɖʐ", "ɟʝ", "ɡɣ", - "ɢʁ", "kp", "ɡb") - special_feats <- read.csv(file.path("..", "raw-data", "FEATURES", - "special-feature-table.csv"), - fileEncoding="UTF-8") - missing <- setdiff(special_cases, special_feats$segment) - expect(length(missing) == 0, - paste("special_cases pair(s) missing from special-feature-table.csv:", - paste(missing, collapse=", "))) }) From 792ea5a5508e5ed0bdc163a1d06610d9ec60a2ec Mon Sep 17 00:00:00 2001 From: varunghat Date: Sun, 23 Aug 2026 14:36:20 +0200 Subject: [PATCH 18/19] Renamed test file suffix from special-casess to c-cedilla --- ...st-regression-special-casess.R => test-regression-c-cedilla.R} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test-regression-special-casess.R => test-regression-c-cedilla.R} (100%) diff --git a/tests/test-regression-special-casess.R b/tests/test-regression-c-cedilla.R similarity index 100% rename from tests/test-regression-special-casess.R rename to tests/test-regression-c-cedilla.R From 9f8864d53d48ae839561c0366ab6020921e16d42 Mon Sep 17 00:00:00 2001 From: varunghat Date: Sun, 23 Aug 2026 15:04:35 +0200 Subject: [PATCH 19/19] Added check special_cases lookup returns exactly one row in add-features.R --- scripts/add-features.R | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/add-features.R b/scripts/add-features.R index f5530b2..da5d865 100644 --- a/scripts/add-features.R +++ b/scripts/add-features.R @@ -144,6 +144,7 @@ make_feat_vec_from_mat <- function(feat_mat) { zero=TRUE) ## look up the combined base glyphs in the special_feats table base_vec <- special_feats[special_feats$segment == base_pair, ] + stopifnot(nrow(base_vec) == 1) ## populate the feature vector from the special_feats table feat_vec[feature_cols] <- base_vec[feature_cols] # delete two bases