Issue 344 tests #385
Issue 344 tests #385
Conversation
zozlak
commented
Dec 21, 2021
zozlak
commented
- Fixed timing in the issue check_interrupts = TRUE causes certain query with inner join to fail with error message "select() on the connection failed" #244 test (the issue affected queries running longer than 1 second)
- Query interrupt test extended with a connection PID-based check if the query has been cancelled also on the server.
The issue 244 concerned queries taking longer than 1 second (queries where the`select()` timeout is reached). Test description and pg_sleep() timining adjusted to test such a scenario.
The query interrupt test extended with a check probing if the request has been cancelled on the database server. The test is based on the PID of the connection so it should be reliable.
| @@ -1,9 +1,9 @@ | |||
| context("checkInterrupts") | |||
|
|
|||
| test_that("check_interrupts = TRUE works with queries < 1 second (#244)", { | |||
| test_that("check_interrupts = TRUE works with queries > 1 second (#244)", { | |||
There was a problem hiding this comment.
The original test is good. Can you please create a new test?
Should the test condition be expect_lt(time[["elapsed"]], 0.9) then?
There was a problem hiding this comment.
If we claim this is a test for #244, it should definitely test a query longer than 1 second because the #244 was caused by a fact that the structure taken by the select() requires different reinitialization under linux and windows, after the select() timeout is reached (and we hardcode the timeout to 1 second).
Of course I can also add another test with query shorter than 1 second, just then we shouldn't claim it tests #244.
There was a problem hiding this comment.
Maybe the reference to #244 is wrong, I'd need to take a closer look. Either way, can you please keep the original test?
There was a problem hiding this comment.
Hm, the test CI failed on the "update snapshots" step - I have no idea what does it mean.
When I run tests locally all pass but (test-checkInterrupts.R:50:3): check_interrupts = TRUE interrupts immediately (#336) (which means the query isn't cancelled on the server side when the interrupt breaks dbGetQuery()).
|
Thanks! |
