Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cypress tests #509

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ tmp.*
vignettes/*.R
vignettes/*.html
vignettes/*.md
tests/node_modules
tests/cypress/videos
2 changes: 1 addition & 1 deletion R/FilterState.R
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ FilterState <- R6::R6Class( # nolint

div(
id = id,
class = "panel filter-card",
class = "panel filter-card cy-filter-card",
include_js_files("count-bar-labels.js"),
div(
class = "filter-card-header",
Expand Down
23 changes: 13 additions & 10 deletions R/FilterStateChoices.R
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ ChoicesFilterState <- R6::R6Class( # nolint
countsmax = countsmax
)
div(
class = "choices_state",
class = "choices_state cy-factor-selection-inputs",
if (private$is_multiple()) {
checkboxGroupInput(
inputId = ns("selection"),
Expand Down Expand Up @@ -399,15 +399,18 @@ ChoicesFilterState <- R6::R6Class( # nolint
countmax = countsmax
)

teal.widgets::optionalSelectInput(
inputId = ns("selection"),
choices = stats::setNames(private$get_choices(), labels),
selected = private$get_selected(),
multiple = private$is_multiple(),
options = shinyWidgets::pickerOptions(
actionsBox = TRUE,
liveSearch = (length(private$get_choices()) > 10),
noneSelectedText = "Select a value"
div(
class = "cy-character-selection-inputs",
teal.widgets::optionalSelectInput(
inputId = ns("selection"),
choices = stats::setNames(private$get_choices(), labels),
selected = private$get_selected(),
multiple = private$is_multiple(),
options = shinyWidgets::pickerOptions(
actionsBox = TRUE,
liveSearch = (length(private$get_choices()) > 10),
noneSelectedText = "Select a value"
)
)
)
}
Expand Down
6 changes: 3 additions & 3 deletions R/FilterStateDate.R
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,13 @@ DateFilterState <- R6::R6Class( # nolint
div(
class = "flex",
actionButton(
class = "date_reset_button",
class = "date_reset_button cy-date-from-reset",
inputId = ns("start_date_reset"),
label = NULL,
icon = icon("fas fa-undo")
),
div(
class = "w-80 filter_datelike_input",
class = "w-80 filter_datelike_input cy-date-inputs",
dateRangeInput(
inputId = ns("selection"),
label = NULL,
Expand All @@ -293,7 +293,7 @@ DateFilterState <- R6::R6Class( # nolint
)
),
actionButton(
class = "date_reset_button",
class = "date_reset_button cy-date-to-reset",
inputId = ns("end_date_reset"),
label = NULL,
icon = icon("fas fa-undo")
Expand Down
14 changes: 10 additions & 4 deletions R/FilterStateDatettime.R
Original file line number Diff line number Diff line change
Expand Up @@ -349,18 +349,24 @@ DatetimeFilterState <- R6::R6Class( # nolint
div(
div(
class = "flex",
ui_reset_1,
div(
class = "cy-datetime-from-reset",
ui_reset_1
),
div(
class = "flex w-80 filter_datelike_input",
div(class = "w-45 text-center", ui_input_1),
div(class = "w-45 text-center cy-datetime-from-input", ui_input_1),
span(
class = "input-group-addon w-10",
span(class = "input-group-text w-100 justify-content-center", "to"),
title = "Times are displayed in the local timezone and are converted to UTC in the analysis"
),
div(class = "w-45 text-center", ui_input_2)
div(class = "w-45 text-center cy-datetime-to-input", ui_input_2)
),
ui_reset_2
div(
class = "cy-datetime-to-reset",
ui_reset_2
)
),
private$keep_na_ui(ns("keep_na"))
)
Expand Down
2 changes: 1 addition & 1 deletion R/FilterStateLogical.R
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ LogicalFilterState <- R6::R6Class( # nolint
}
div(
div(
class = "choices_state",
class = "choices_state cy-logical-selection-inputs",
uiOutput(ns("trigger_visible"), inline = TRUE),
ui_input
),
Expand Down
19 changes: 11 additions & 8 deletions R/FilterStateRange.R
Original file line number Diff line number Diff line change
Expand Up @@ -402,14 +402,17 @@ RangeFilterState <- R6::R6Class( # nolint
ui_inputs = function(id) {
ns <- NS(id)
shiny::isolate({
ui_input <- shinyWidgets::numericRangeInput(
inputId = ns("selection_manual"),
label = NULL,
min = private$get_choices()[1L],
max = private$get_choices()[2L],
value = private$get_selected(),
step = private$numeric_step,
width = "100%"
ui_input <- div(
class = "cy-numeric-selection-inputs",
shinyWidgets::numericRangeInput(
inputId = ns("selection_manual"),
label = NULL,
min = private$get_choices()[1L],
max = private$get_choices()[2L],
value = private$get_selected(),
step = private$numeric_step,
width = "100%"
)
)
tagList(
div(
Expand Down
2 changes: 1 addition & 1 deletion R/FilteredData.R
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ FilteredData <- R6::R6Class( # nolint
div(
id = ns("filters_overview_contents"),
div(
class = "teal_active_summary_filter_panel",
class = "teal_active_summary_filter_panel cy-active-summary-table",
tableOutput(ns("table"))
)
)
Expand Down
12 changes: 12 additions & 0 deletions tests/cypress.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const { defineConfig } = require("cypress");

module.exports = defineConfig({
video: true,
viewportWidth: 1000,
viewportHeight: 1200,
e2e: {
setupNodeEvents(on, config) {
// implement node event listeners here
},
},
});
36 changes: 36 additions & 0 deletions tests/cypress/app.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
library(teal)

data <- teal_data() |>
within({
set.seed(123)
size <- 20
data_frame <- data.frame(
id = seq(size),
numeric = round(rnorm(size, 25, 10)),
logical = sample(c(TRUE, FALSE), size = size, replace = TRUE),
factor = factor(sample(LETTERS[1:4], size = size, replace = TRUE)),
character = sample(letters, size = size, replace = TRUE),
datetime = sample(
seq(as.POSIXct("2000/01/01"), as.POSIXct("2024/01/01"), by = "day"),
size = size
)
)
iris <- head(iris, 20)
data_frame$date <- as.Date(data_frame$datetime)
})
datanames(data) <- c("data_frame", "iris")
app <- teal::init(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be a generic shiny app instead of teal. Testing could be distorted by teal. I suggest to use shiny app from vignette. Should also test the output if and how data gets filtererd?

data = data,
modules = modules(example_module()),
filter = teal_slices(
teal_slice("data_frame", "numeric"),
teal_slice("data_frame", "logical"),
teal_slice("data_frame", "factor"),
teal_slice("data_frame", "character"),
teal_slice("data_frame", "datetime"),
teal_slice("data_frame", "date")
)
)

options(shiny.port = 5555)
shinyApp(app$ui, app$server)
150 changes: 150 additions & 0 deletions tests/cypress/e2e/filters-data.frame.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
describe("data.frame filters", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for cypress tests you assume that there is a specific class id/name, so that's why you put all those divs( around in R code. Then you need to have node and know JavaScript to write tests? Is that right? Is JS part of CoreDev regular skillset?

beforeEach(() => {
cy.visit("http://127.0.0.1:5555/");
cy.waitForStabilityAndCatchError("body");
cy.get(".cy-active-summary-table tbody tr").as("activeSummary");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the fact id is duplicated with "cy_" prefix. I propose following convention: since we need to be aware that certain element is tested let's just add cy-testable class to the existing class (as a second class). Then selector for this like will be

Suggested change
cy.get(".cy-active-summary-table tbody tr").as("activeSummary");
cy.get(".cy-testable.teal_active_summary_filter_panel tbody tr").as("activeSummary");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽 Let's try to find out if it's possible to implement the tests without any change to the classes in the package as we talked about. If unsuccessful, we can use this convention, I like it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I respect the idea of having a distinction between what's testable and what is not so app developer won't change this casually (I hope at least). It will be good to write down this convention somewhere that "testable" means don't touch it ;)

Also, I think we need to review all html classes to follow some "template". They seem to be random

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we need to review all html classes to follow some "template". They seem to be random

Agreed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if testable is propagated in some good-practice guidelines then this should be ok

});

it("should filter proper values for numeric filters", () => {
cy.get(".cy-filter-card").contains("numeric").as("numericCard");
cy.get("@numericCard").click();
cy.waitForStabilityAndCatchError("body");

cy.get("@numericCard")
.get(".cy-numeric-selection-inputs input:first")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking about parent-child naming convention for filters. In FilterState card can have a general class like filter-card, then RangeFilterState$ui_inputs will just have .numeric-filter-inputs.

Suggested change
.get(".cy-numeric-selection-inputs input:first")
.get(".filter-card > .numeric-filter > .cy-testable.filter-card input:first")

Let's sit down and design a complete wireframe for teal/teal.slice and let's think about classes/ids of each element

.clear()
.type("10{enter}");
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "19/20");

cy.get("@numericCard")
.get(".cy-numeric-selection-inputs input:last")
.clear()
.type("20{enter}");
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "5/20");
});

it("should filter proper values for logical filters", () => {
cy.get(".cy-filter-card").contains("logical").as("logicalCard");
cy.get("@logicalCard").click();
cy.waitForStabilityAndCatchError("body");

cy.get("@logicalCard")
.get(".cy-logical-selection-inputs .checkbox")
.contains("TRUE")
.as("trueCheckbox");
cy.get("@logicalCard")
.get(".cy-logical-selection-inputs .checkbox")
.contains("FALSE")
.as("falseCheckbox");

cy.get("@trueCheckbox").click();
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "7/20");

cy.get("@falseCheckbox").click();
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "0/20");

cy.get("@trueCheckbox").click();
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "13/20");
});

it("should filter proper values for factor filters", () => {
cy.get(".cy-filter-card").contains("factor").as("factorCard");
cy.get("@factorCard").click();
cy.waitForStabilityAndCatchError("body");

cy.get("@factorCard")
.get(".cy-factor-selection-inputs .checkbox")
.contains("C")
.click();
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "15/20");
});

it("should filter proper values for character filters", () => {
cy.get(".cy-filter-card").contains("character").as("characterCard").click();
cy.waitForStabilityAndCatchError("body");

cy.get("@characterCard")
.get(".cy-character-selection-inputs .dropdown-toggle")
.as("characterInputs")
.click();
cy.waitForStabilityAndCatchError("body");

cy.get("@characterInputs").get("li").contains("b").click();
cy.waitForStabilityAndCatchError("body");
cy.get("body").type("{esc}");
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "19/20");
});

it("should filter proper values for datetime filters", () => {
cy.get(".cy-filter-card").contains("datetime").as("datetimeCard").click();
cy.waitForStabilityAndCatchError("body");

cy.get("@datetimeCard").get(".cy-datetime-from-input").click();
cy.waitForStabilityAndCatchError("body");
cy.get(
'.air-datepicker-global-container .air-datepicker-cell[data-year="2001"][data-month="10"][data-date="29"]'
).click();
cy.get("body").type("{esc}");
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "19/20");

cy.get("@datetimeCard").get(".cy-datetime-to-input").click();
cy.get(
'.air-datepicker-global-container .air-datepicker-cell[data-year="2021"][data-month="5"][data-date="1"]'
).click();
cy.get("body").type("{esc}");
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "18/20");
});

it("should filter proper values for date filters", () => {
cy.get(".cy-filter-card")
.contains(/^date$/)
.as("dateCard")
.click();
cy.waitForStabilityAndCatchError("body");

cy.get("@dateCard").get(".cy-date-inputs input:first").click();
cy.waitForStabilityAndCatchError("body");

cy.get(".datepicker-days table td").contains("16").click(),
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "19/20");

cy.get("@dateCard").get(".cy-date-inputs input:last").click();
cy.waitForStabilityAndCatchError("body");

cy.get(".datepicker-days table td").contains("5").click(),
cy.waitForStabilityAndCatchError("body");
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "18/20");
});
});
13 changes: 13 additions & 0 deletions tests/cypress/e2e/filters-general.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
describe("General filter panel features", () => {
beforeEach(() => {
cy.visit("http://127.0.0.1:5555/");
cy.waitForStabilityAndCatchError("body");
cy.get(".cy-active-summary-table tbody tr").as("activeSummary");
});

it("should initiate filter panel with the right values", () => {
cy.get("@activeSummary")
.should("contain", "data_frame")
.should("contain", "20/20");
});
});
33 changes: 33 additions & 0 deletions tests/cypress/support/commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Cypress.Commands.add(
"waitForStabilityAndCatchError",
(selector, stabilityPeriod = 300) => {
let lastInnerHTML = "";
let timesRun = 0;
const checkInterval = 100;
const maxTimesRun = stabilityPeriod / checkInterval;

function checkForChanges() {
cy.get(selector).then(($el) => {
// Check for shiny-output-error class anywhere in the body
if (Cypress.$("body").find(".shiny-output-error").length > 0) {
throw new Error(
"shiny-output-error class detected during stability check"
);
}

const currentInnerHTML = $el.prop("innerHTML");
if (currentInnerHTML !== lastInnerHTML) {
lastInnerHTML = currentInnerHTML;
timesRun = 0;
} else if (timesRun < maxTimesRun) {
timesRun += 1;
} else {
return;
}
cy.wait(checkInterval).then(checkForChanges);
});
}

checkForChanges();
}
);
Loading