From 2f68768082753cf23d4001d2c3c62ce700420704 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 15 Dec 2023 09:03:13 -0800 Subject: [PATCH 1/4] refactor test-translate - translation pair of success/failure tests moved into `expect_set_translate()` function in helper-translate.R - expect_false(identical()) changed to expect_failure(expect_setequal()) to produce more informative error messages when they occur --- tests/testthat/helper-translate.R | 69 +++++++++++++ tests/testthat/test-translate.R | 161 +++++++----------------------- 2 files changed, 104 insertions(+), 126 deletions(-) create mode 100644 tests/testthat/helper-translate.R diff --git a/tests/testthat/helper-translate.R b/tests/testthat/helper-translate.R new file mode 100644 index 000000000..452f4a42f --- /dev/null +++ b/tests/testthat/helper-translate.R @@ -0,0 +1,69 @@ +#' Expect translations are rendered properly in the HTML page +#' +#' One of the challenges of translations is that we need to test both that the +#' translation works and that the default text does not work. +#' +#' We are using the `tr_()` function to ensure that our translations are +#' working, so we expect that we can use the following to test that the +#' translation works: +#' +#' ``` +#' expected <- withr::with_language("ja", tr_("Summary and Setup")) +#' expect_equal(xml2::xml_text(node, trim = TRUE), expected) +#' ``` +#' +#' This logic is not wrong, but it will almost always give a successful result +#' because if the `tr_()` function does not find a translation, it will return +#' the same string. Thus, we need to pair this with an expectation that we know +#' should fail---translating the default state for English: +#' +#' ``` +#' unexpected <- withr::with_language("en", tr_("Summary and Setup")) +#' expect_failure({ +#' expect_equal(xml2::xml_text(node, trim = TRUE), unexpected) +#' }) +#' ``` +#' +#' It's with this pair of expectations that we can confirm that a translation +#' is working correctly. This function also expands this idea to compare sets of +#' translations when there may be duplicates. +#' +#' @param node an xml_node or xml_nodeset object +#' @param strings a character vector of _untranslated_ strings +#' @param language a character vector of length 1 defining the language to use +#' for translations +#' @noRd +#' @examples +#' sitepath <- fs::path(lsn, "site/docs") +#' profiles <- xml2::read_html(fs::path(sitepath, "profiles.html")) +#' h1_profiles <- xml2::xml_find_first(profiles, "//main/div/h1") +#' expect_set_translated(h1_profiles, "Learner Profiles") +expect_set_translated <- function(node, strings, language = "ja") { + # translate a vector into the defined langage and english for comparison + expected_strings <- withr::with_language(language, { + unname(vapply(strings, tr_, character(1))) + }) + en_strings <- withr::with_language("en", { + unname(vapply(strings, tr_, character(1))) + }) + actual <- xml2::xml_text(node, trim = TRUE) + + # The translations should work. + # NOTE: this will work even if the translation function is not working and + # falls back to English. + # --- reasons why this would not work: + # --- 1. the language of the web page is different than the language + # --- 2. the web page has no text + # --- 3. the tr_() function returns NA + expect_setequal(actual, expected = expected_strings) + + # The English should NOT work + # --- reasons why this would not work: + # --- 1. the tr_() function is not actually performing any translations + expect_failure({ + expect_setequal(actual, expected = en_strings) + }) +} + + + diff --git a/tests/testthat/test-translate.R b/tests/testthat/test-translate.R index 0f5d30054..e2675047b 100644 --- a/tests/testthat/test-translate.R +++ b/tests/testthat/test-translate.R @@ -6,8 +6,6 @@ config$lang <- "ja" yaml::write_yaml(config, config_path) sitepath <- fs::path(tmp, "site", "docs") - - test_that("set_language() uses english by default", { os <- tolower(Sys.info()[["sysname"]]) @@ -17,12 +15,12 @@ test_that("set_language() uses english by default", { # default is english set_language() expect_equal(tr_("OUTPUT"), "OUTPUT") - + # set to japanese and it becomes different set_language("ja") OUTJA <- tr_("OUTPUT") expect_false(identical(OUTJA, "OUTPUT")) - + # unknown language will not switch the current language suppressMessages(expect_message(set_language("xx"), "languages")) expect_equal(tr_("OUTPUT"), OUTJA) @@ -47,7 +45,7 @@ test_that("set_language() can use country codes", { # the country codes will fall back to language code if they don't exist expect_silent(set_language("es")) expect_equal(tr_("OUTPUT"), OUTAR) - + }) @@ -70,6 +68,9 @@ test_that("is_known_language returns a warning for an unknown language", { test_that("Lessons can be translated with lang setting", { + # NOTE: this requires the expect_set_translated() function defined in + # tests/testthat/helper-translate.R + skip_if_not(rmarkdown::pandoc_available("2.11")) os <- tolower(Sys.info()[["sysname"]]) @@ -78,33 +79,16 @@ test_that("Lessons can be translated with lang setting", { # Build lesson suppressMessages(build_lesson(tmp, preview = FALSE, quiet = TRUE)) - + # GENERATED PAGES ------------------------------------------------ # Check generated page headers inst_note <- xml2::read_html(fs::path(sitepath, "instructor/instructor-notes.html")) h1_inst <- xml2::xml_find_first(inst_note, "//main/div/h1") - expect_equal( - xml2::xml_text(h1_inst), - withr::with_language("ja", tr_("Instructor Notes")) - ) - expect_false( - identical( - xml2::xml_text(h1_inst), - withr::with_language("en", tr_("Instructor Notes")) - ) - ) + expect_set_translated(h1_inst, "Instructor Notes") + profiles <- xml2::read_html(fs::path(sitepath, "profiles.html")) h1_profiles <- xml2::xml_find_first(profiles, "//main/div/h1") - expect_equal( - xml2::xml_text(h1_profiles), - withr::with_language("ja", tr_("Learner Profiles")) - ) - expect_false( - identical( - xml2::xml_text(h1_profiles), - withr::with_language("en", tr_("Learner Profiles")) - ) - ) + expect_set_translated(h1_profiles, "Learner Profiles") # Extract first header (Summary and Setup) from index @@ -116,47 +100,26 @@ test_that("Lessons can be translated with lang setting", { expect_equal(xml2::xml_attr(xml, "lang"), "ja") # Header should be translated to Japanese - expect_true( - identical( - xml2::xml_text(h1_header), - withr::with_language("ja", tr_("Summary and Setup")) - ) - ) + expect_set_translated(h1_header, "Summary and Setup") + # Navbar has expected text - expect_equal( - xml2::xml_text(nav_links), - withr::with_language("ja", - c(tr_("Key Points"), tr_("Glossary"), tr_("Learner Profiles")) - ) - ) - expect_false( - identical( - xml2::xml_text(nav_links), - withr::with_language("en", - c(tr_("Key Points"), tr_("Glossary"), tr_("Learner Profiles")) - ) - ) + expect_set_translated(nav_links, + c("Key Points", "Glossary", "Learner Profiles") ) - # Header should no longer be in English - expect_false( - identical( - xml2::xml_text(h1_header), - withr::with_language("en", tr_("Summary and Setup")) - ) - ) - # aria labels should be translated - arias <- c("Main Navigation", "Toggle Navigation", "Search", "search button", - "Lesson Progress", "close menu", "Next Chapter", "anchor", "Back To Top") - ja_arias <- withr::with_language("ja", vapply(arias, tr_, character(1))) - - expect_false(identical(arias, ja_arias)) - - expect_setequal( - ja_arias, - xml2::xml_text(xml2::xml_find_all(xml, ".//@aria-label")) - ) + aria_text <- c("Main Navigation", + "Toggle Navigation", + "Search", + "search button", + "Lesson Progress", + "close menu", + "Next Chapter", + "anchor", + "Back To Top" + ) + aria_labels <- xml2::xml_find_all(xml, ".//@aria-label") + expect_set_translated(aria_labels, aria_text) # Episode elements ------------------------------------------------- # We use here the Instructor view because it is more fully featured @@ -164,19 +127,8 @@ test_that("Lessons can be translated with lang setting", { nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]") # navbar has expected text - expect_equal( - xml2::xml_text(nav_links), - withr::with_language("ja", - c(tr_("Key Points"), tr_("Instructor Notes"), tr_("Extract All Images")) - ) - ) - expect_false( - identical( - xml2::xml_text(nav_links), - withr::with_language("en", - c(tr_("Key Points"), tr_("Instructor Notes"), tr_("Extract All Images")) - ) - ) + expect_set_translated(nav_links, + c("Key Points", "Instructor Notes", "Extract All Images") ) # overview, objectives, and questions @@ -184,57 +136,22 @@ test_that("Lessons can be translated with lang setting", { # Overview card overview <- xml2::xml_find_first(overview_card, ".//h2[@class='card-header']") - expect_equal( - xml2::xml_text(overview, trim = TRUE), - withr::with_language("ja", tr_("Overview")) - ) - expect_false( - identical( - xml2::xml_text(overview, trim = TRUE), - withr::with_language("en", tr_("Overview")) - ) - ) + expect_set_translated(overview, "Overview") # Questions and Objectives quob <- xml2::xml_find_all(overview_card, ".//h3[@class='card-title']") - expect_equal( - xml2::xml_text(quob, trim = TRUE), - withr::with_language("ja", c(tr_("Questions"), tr_("Objectives"))) - ) - expect_false( - identical( - xml2::xml_text(quob, trim = TRUE), - withr::with_language("en", c(tr_("Questions"), tr_("Objectives"))) - ) - ) + expect_set_translated(quob, c("Questions", "Objectives")) + # Keypoints are always the last block and should be auto-translated xpath_keypoints <- ".//div[@class='callout keypoints']//h3[@class='callout-title']" keypoints <- xml2::xml_find_first(xml, xpath_keypoints) - expect_equal( - xml2::xml_text(keypoints, trim = TRUE), - withr::with_language("ja", tr_("Key Points")) - ) - expect_false( - identical( - xml2::xml_text(keypoints, trim = TRUE), - withr::with_language("en", tr_("Key Points")) - ) - ) + expect_set_translated(keypoints, "Key Points") # Instructor note headings should be translated xpath_instructor <- ".//div[@class='accordion-item']/button/h3" instructor_note <- xml2::xml_find_all(xml, xpath_instructor) - expect_equal( - xml2::xml_text(instructor_note, trim = TRUE), - withr::with_language("ja", tr_("Instructor Note")) - ) - expect_false( - identical( - xml2::xml_text(instructor_note, trim = TRUE), - withr::with_language("en", tr_("Instructor Note")) - ) - ) + expect_set_translated(instructor_note, "Instructor Note") # solution headings should be translated xpath_solution <- ".//div[@class='accordion-item']/button/h4" @@ -242,15 +159,7 @@ test_that("Lessons can be translated with lang setting", { # take the last solution block because that's the one that does not have # a title. solution <- solution[[length(solution)]] - expect_equal( - xml2::xml_text(solution, trim = TRUE), - withr::with_language("ja", tr_("Show me the solution")) - ) - expect_false( - identical( - xml2::xml_text(solution, trim = TRUE), - withr::with_language("en", tr_("Show me the solution")) - ) - ) + + expect_set_translated(solution, "Show me the solution") }) From 2e0fcd2ab0235ebbe55f47382d03d50896feefda Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 15 Dec 2023 10:12:21 -0800 Subject: [PATCH 2/4] rearrange tests in page types --- tests/testthat/test-translate.R | 61 +++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/tests/testthat/test-translate.R b/tests/testthat/test-translate.R index e2675047b..be7d27c6d 100644 --- a/tests/testthat/test-translate.R +++ b/tests/testthat/test-translate.R @@ -80,35 +80,24 @@ test_that("Lessons can be translated with lang setting", { # Build lesson suppressMessages(build_lesson(tmp, preview = FALSE, quiet = TRUE)) - # GENERATED PAGES ------------------------------------------------ - # Check generated page headers - inst_note <- xml2::read_html(fs::path(sitepath, "instructor/instructor-notes.html")) - h1_inst <- xml2::xml_find_first(inst_note, "//main/div/h1") - expect_set_translated(h1_inst, "Instructor Notes") - - profiles <- xml2::read_html(fs::path(sitepath, "profiles.html")) - h1_profiles <- xml2::xml_find_first(profiles, "//main/div/h1") - expect_set_translated(h1_profiles, "Learner Profiles") - - - # Extract first header (Summary and Setup) from index + # Home Page ------------------------------------------------------ xml <- xml2::read_html(fs::path(sitepath, "index.html")) - h1_header <- xml2::xml_find_all(xml, "//h1[@class='schedule-heading']") - nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]") - # language should be set to japanese expect_equal(xml2::xml_attr(xml, "lang"), "ja") - # Header should be translated to Japanese + # Extract first header (Summary and Setup) from index + h1_header <- xml2::xml_find_all(xml, "//h1[@class='schedule-heading']") expect_set_translated(h1_header, "Summary and Setup") # Navbar has expected text + nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]") expect_set_translated(nav_links, c("Key Points", "Glossary", "Learner Profiles") ) # aria labels should be translated - aria_text <- c("Main Navigation", + aria_text <- c( + "Main Navigation", "Toggle Navigation", "Search", "search button", @@ -121,6 +110,18 @@ test_that("Lessons can be translated with lang setting", { aria_labels <- xml2::xml_find_all(xml, ".//@aria-label") expect_set_translated(aria_labels, aria_text) + + # GENERATED PAGES ------------------------------------------------ + # Check generated page headers + inst_note <- xml2::read_html(fs::path(sitepath, "instructor/instructor-notes.html")) + h1_inst <- xml2::xml_find_first(inst_note, "//main/div/h1") + expect_set_translated(h1_inst, "Instructor Notes") + + profiles <- xml2::read_html(fs::path(sitepath, "profiles.html")) + h1_profiles <- xml2::xml_find_first(profiles, "//main/div/h1") + expect_set_translated(h1_profiles, "Learner Profiles") + + # Episode elements ------------------------------------------------- # We use here the Instructor view because it is more fully featured xml <- xml2::read_html(fs::path(sitepath, "instructor", "introduction.html")) @@ -131,17 +132,25 @@ test_that("Lessons can be translated with lang setting", { c("Key Points", "Instructor Notes", "Extract All Images") ) + # aria labels should be translated + aria_text <- c( + "Main Navigation", + "Toggle Navigation", + "Search", + "search button", + "Lesson Progress", + "close menu", + "Previous and Next Chapter", + "anchor", + "Back To Top" + ) + aria_labels <- xml2::xml_find_all(xml, ".//@aria-label") + expect_set_translated(aria_labels, aria_text) + # overview, objectives, and questions overview_card <- xml2::xml_find_first(xml, ".//div[@class='overview card']") - - # Overview card - overview <- xml2::xml_find_first(overview_card, ".//h2[@class='card-header']") - expect_set_translated(overview, "Overview") - - # Questions and Objectives - quob <- xml2::xml_find_all(overview_card, ".//h3[@class='card-title']") - expect_set_translated(quob, c("Questions", "Objectives")) - + over_heads <- xml2::xml_find_all(overview_card, ".//h2 | .//h3") + expect_set_translated(over_heads, c("Overview", "Questions", "Objectives")) # Keypoints are always the last block and should be auto-translated xpath_keypoints <- ".//div[@class='callout keypoints']//h3[@class='callout-title']" From dc0d2112a09812f0f4ae8fa6486c13580df3f525 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 15 Dec 2023 10:46:21 -0800 Subject: [PATCH 3/4] add helpers for title elements; test 404 page --- tests/testthat/helper-translate.R | 27 +++++++++++++++++++ tests/testthat/test-translate.R | 45 ++++++++++++++++++++++++------- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/tests/testthat/helper-translate.R b/tests/testthat/helper-translate.R index 452f4a42f..7250fec0b 100644 --- a/tests/testthat/helper-translate.R +++ b/tests/testthat/helper-translate.R @@ -65,5 +65,32 @@ expect_set_translated <- function(node, strings, language = "ja") { }) } +expect_h1_translated <- function(doc, string, language = "ja") { + node <- xml2::xml_find_first(doc, "//main/div/h1") + expect_set_translated(node, string, language) +} +expect_title_translated <- function(doc, string, language = "ja") { + # translate a vector into the defined langage and english for comparison + # NOTE: we need to set the regex to end with the string in question + expected_string <- paste0("\\: ", withr::with_language("ja", tr_(string)), "$") + en_string <- paste0("\\: ", withr::with_language("en", tr_(string)), "$") + node <- xml2::xml_find_first(doc, "//head/title") + actual <- xml2::xml_text(node, trim = TRUE) + + # The translations should work. + # NOTE: this will work even if the translation function is not working and + # falls back to English. + # --- reasons why this would not work: + # --- 1. the language of the web page is different than the language + # --- 2. the web page has no text + # --- 3. the tr_() function returns NA + expect_match(actual, regex = expected_string) + + # The English should NOT work + # --- reasons why this would not work: + # --- 1. the tr_() function is not actually performing any translations + expect_no_match(actual, regex = en_string) + +} diff --git a/tests/testthat/test-translate.R b/tests/testthat/test-translate.R index be7d27c6d..587069217 100644 --- a/tests/testthat/test-translate.R +++ b/tests/testthat/test-translate.R @@ -80,20 +80,34 @@ test_that("Lessons can be translated with lang setting", { # Build lesson suppressMessages(build_lesson(tmp, preview = FALSE, quiet = TRUE)) + # Home Page ------------------------------------------------------ + # Testing both learner and instructor versions of this page xml <- xml2::read_html(fs::path(sitepath, "index.html")) + instruct <- xml2::read_html(fs::path(sitepath, "instructor/index.html")) # language should be set to japanese expect_equal(xml2::xml_attr(xml, "lang"), "ja") + expect_equal(xml2::xml_attr(instruct, "lang"), "ja") + expect_title_translated(xml, "Summary and Setup") + expect_title_translated(instruct, "Summary and Schedule") # Extract first header (Summary and Setup) from index - h1_header <- xml2::xml_find_all(xml, "//h1[@class='schedule-heading']") + h1_xpath <- "//h1[@class='schedule-heading']" + h1_header <- xml2::xml_find_all(xml, h1_xpath) expect_set_translated(h1_header, "Summary and Setup") + ih1_header <- xml2::xml_find_all(instruct, h1_xpath) + expect_set_translated(ih1_header, "Summary and Schedule") # Navbar has expected text - nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]") + nav_xpath <- "//a[starts-with(@class,'nav-link')]" + nav_links <- xml2::xml_find_all(xml, nav_xpath) expect_set_translated(nav_links, c("Key Points", "Glossary", "Learner Profiles") ) + inav_links <- xml2::xml_find_all(instruct, nav_xpath) + expect_set_translated(inav_links, + c("Key Points", "Instructor Notes", "Extract All Images") + ) # aria labels should be translated aria_text <- c( @@ -112,22 +126,35 @@ test_that("Lessons can be translated with lang setting", { # GENERATED PAGES ------------------------------------------------ - # Check generated page headers - inst_note <- xml2::read_html(fs::path(sitepath, "instructor/instructor-notes.html")) - h1_inst <- xml2::xml_find_first(inst_note, "//main/div/h1") - expect_set_translated(h1_inst, "Instructor Notes") + # These pages should have translated headers and title elements + inst_notes_path <- fs::path(sitepath, "instructor/instructor-notes.html") + inst_notes <- xml2::read_html(inst_notes_path) + expect_equal(xml2::xml_attr(inst_notes, "lang"), "ja") + expect_h1_translated(inst_notes, "Instructor Notes") + expect_title_translated(inst_notes, "Instructor Notes") profiles <- xml2::read_html(fs::path(sitepath, "profiles.html")) - h1_profiles <- xml2::xml_find_first(profiles, "//main/div/h1") - expect_set_translated(h1_profiles, "Learner Profiles") + expect_equal(xml2::xml_attr(profiles, "lang"), "ja") + expect_h1_translated(profiles, "Learner Profiles") + expect_title_translated(profiles, "Learner Profiles") + + fof <- xml2::read_html(fs::path(sitepath, "404.html")) + expect_equal(xml2::xml_attr(fof, "lang"), "ja") + expect_h1_translated(fof, "Page not found") + expect_title_translated(fof, "Page not found") + + imgs <- xml2::read_html(fs::path(sitepath, "instructor/images.html")) + expect_equal(xml2::xml_attr(imgs, "lang"), "ja") + expect_title_translated(imgs, "All Images") # Episode elements ------------------------------------------------- # We use here the Instructor view because it is more fully featured xml <- xml2::read_html(fs::path(sitepath, "instructor", "introduction.html")) - nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]") + expect_equal(xml2::xml_attr(xml, "lang"), "ja") # navbar has expected text + nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]") expect_set_translated(nav_links, c("Key Points", "Instructor Notes", "Extract All Images") ) From 0349a41ed6a3df8955992090980ca7a08beecd23 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 15 Dec 2023 11:13:15 -0800 Subject: [PATCH 4/4] add tests for navigation --- tests/testthat/test-translate.R | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/testthat/test-translate.R b/tests/testthat/test-translate.R index 587069217..9f0bf61d9 100644 --- a/tests/testthat/test-translate.R +++ b/tests/testthat/test-translate.R @@ -87,6 +87,11 @@ test_that("Lessons can be translated with lang setting", { instruct <- xml2::read_html(fs::path(sitepath, "instructor/index.html")) # language should be set to japanese expect_equal(xml2::xml_attr(xml, "lang"), "ja") + to_main <- xml2::xml_find_first(xml, "//a[@href='#main-content']") + ito_main <- xml2::xml_find_first(instruct, "//a[@href='#main-content']") + expect_set_translated(to_main, "Skip to main content") + expect_set_translated(ito_main, "Skip to main content") + expect_equal(xml2::xml_attr(instruct, "lang"), "ja") expect_title_translated(xml, "Summary and Setup") expect_title_translated(instruct, "Summary and Schedule") @@ -98,6 +103,10 @@ test_that("Lessons can be translated with lang setting", { ih1_header <- xml2::xml_find_all(instruct, h1_xpath) expect_set_translated(ih1_header, "Summary and Schedule") + # Schedule for instructor view ends with "Finish" + final_cell <- xml2::xml_find_first(instruct, "//tr[last()]/td[2]") + expect_set_translated(final_cell, "Finish") + # Navbar has expected text nav_xpath <- "//a[starts-with(@class,'nav-link')]" nav_links <- xml2::xml_find_all(xml, nav_xpath) @@ -122,7 +131,9 @@ test_that("Lessons can be translated with lang setting", { "Back To Top" ) aria_labels <- xml2::xml_find_all(xml, ".//@aria-label") + iaria_labels <- xml2::xml_find_all(instruct, ".//@aria-label") expect_set_translated(aria_labels, aria_text) + expect_set_translated(iaria_labels, aria_text) # GENERATED PAGES ------------------------------------------------ @@ -150,8 +161,13 @@ test_that("Lessons can be translated with lang setting", { # Episode elements ------------------------------------------------- # We use here the Instructor view because it is more fully featured + # NOTE: we expect this to be the first episode after the home page xml <- xml2::read_html(fs::path(sitepath, "instructor", "introduction.html")) expect_equal(xml2::xml_attr(xml, "lang"), "ja") + to_main <- xml2::xml_find_first(xml, "//a[@href='#main-content']") + expect_set_translated(to_main, "Skip to main content") + previous <- xml2::xml_find_all(xml, "//a[@class='chapter-link']") + expect_set_translated(previous, c("Home", "Previous")) # navbar has expected text nav_links <- xml2::xml_find_all(xml, "//a[starts-with(@class,'nav-link')]")