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

New test failures with R 3.6 #384

Closed
jimhester opened this issue May 6, 2019 · 12 comments
Closed

New test failures with R 3.6 #384

jimhester opened this issue May 6, 2019 · 12 comments
Labels
bug an unexpected problem or unintended behavior help wanted ❤️ we'd love your help!

Comments

@jimhester
Copy link
Member

It looks like there are a number of tests failures exposed by R 3.6, we should get these fixed.

@jimhester jimhester added bug an unexpected problem or unintended behavior help wanted ❤️ we'd love your help! labels May 6, 2019
@dschlaep
Copy link

#377 is likely one of them (it has R-devel in the title, but that version is by now R v3.6)

@russHyde
Copy link
Collaborator

russHyde commented Aug 12, 2019

Had a look at this under R 3.6.1 (with current lintr master, xml2=1.2.2, rcpp=1.0.2, libxml=2.9.9)

Here master is b5dff71

# In lintr dir on branch matching master
Rscript -e "devtools::load_all(); testthat::test_dir('tests/testthat', reporter = 'summary')" > test_failures.txt

The first 10 failing tests were in these files:

absolute_path_linter: ...................................................
assignment_linter: ....1
clear_cache: ...
load_cache: ...
save_cache: ...........
cache_file: ..
retrieve_file: ..
cache_lint: ..
retrieve_lint: ........
has_lint: ...
find_new_line: .........
lint with cache: ............
checkstyle_output: .
closed_curly_linter: ............
commas_linter: ..............
commented_code_linter: ............2
comments: ...
defaults: ...........................
deprecated: ..........
equals_na_linter: ...3
error: ....
parse_exclusions: .......
normalize_exclusions: ................
exclude: 4
expect_lint: .5..67
extraction_operator_linter: .......................
function_left_parentheses_linter: .......................
get_source_expression: ...............
implicit_integer_linter: .......................
infix_spaces_linter: ...........8
knitr_formats: 9abcde...

Here's an example failure:

── 1. Error: returns the correct linting (@test-assignment_linter.R#12)  ───────
Opening and ending tag mismatch: expr line 8 and equal_assign [76]
1: expect_lint("blah=1", rex("Use <-, not =, for assignment."), assignment_linter) at tests/testthat/test-assignment_linter.R:12
2: lint(file, ...) at /home/ah327h/progs/pull_requests/lintr/R/expect_lint.R:45
3: get_source_expressions(filename) at /home/ah327h/progs/pull_requests/lintr/R/lint.R:36
4: lapply(top_level_expressions(parsed_content), function(loc) {
       line_nums <- parsed_content$line1[loc]:parsed_content$line2[loc]
       expr_lines <- source_file$lines[line_nums]
       names(expr_lines) <- line_nums
       content <- get_content(expr_lines, parsed_content[loc, ])
       id <- as.character(parsed_content$id[loc])
       edges <- component_edges(tree, id)
       pc <- parsed_content[c(loc, edges), ]
       list(filename = filename, line = parsed_content[loc, "line1"], column = parsed_content[loc, 
           "col1"], lines = expr_lines, parsed_content = pc, xml_parsed_content = xml2::read_xml(xmlparsedata::xml_parse_data(pc)), 
           content = content, find_line = find_line_fun(content), find_column = find_column_fun(content))
   }) at /home/ah327h/progs/pull_requests/lintr/R/get_source_expressions.R:83
5: FUN(X[[i]], ...)
6: xml2::read_xml(xmlparsedata::xml_parse_data(pc)) at /home/ah327h/progs/pull_requests/lintr/R/get_source_expressions.R:92
7: read_xml.character(xmlparsedata::xml_parse_data(pc))
8: read_xml.raw(charToRaw(enc2utf8(x)), "UTF-8", ..., as_html = as_html, options = options)
9: doc_parse_raw(x, encoding = encoding, base_url = base_url, as_html = as_html, options = options)

All the other test-failures that came up in those first 10 tests died with the same lines:

6: xml2::read_xml(xmlparsedata::xml_parse_data(pc)) at /home/ah327h/progs/pull_requests/lintr/R/get_source_expressions.R:92
7: read_xml.character(xmlparsedata::xml_parse_data(pc))
8: read_xml.raw(charToRaw(enc2utf8(x)), "UTF-8", ..., as_html = as_html, options = options)
9: doc_parse_raw(x, encoding = encoding, base_url = base_url, as_html = as_html, options = options)

So. In my R 3.6.1 env I tried running the same tests after downgrading xml2 and Rcpp to match the versions I had in an R 3.5.1 env where all the lintr tests pass (Rcpp=1.0.1, xml2=1.2.0; the R 3.5.1 env also has libxml2=2.9.9, so matches my R 3.6.1 env for that at least). The same test failures were thrown.

Will investigate further

@russHyde
Copy link
Collaborator

My debugging example looks like this:

expect_lint("blah=1", rex("Use <-, not =, for assignment."), assignment_linter)
Error in doc_parse_raw(x, encoding = encoding, base_url = base_url, as_html = as_html,  : 
  Opening and ending tag mismatch: expr line 8 and equal_assign [76]

This throws the doc_parse_raw error above

xml2:::read_xml.character calls xml2:::read_xml.raw calls xml2:::doc_parse_raw

With this example, and debug(xml2:::read_xml.character):

> debug(xml2:::read_xml.character)
> expect_lint("blah=1", rex("Use <-, not =, for assignment."), assignment_linter)
debugging in: read_xml.character(my_xml)
debug: {
    if (length(x) == 0) {
        stop("Document is empty", call. = FALSE)
    }
    options <- parse_options(options, xml_parse_options())
    if (grepl("<|>", x)) {
        read_xml.raw(charToRaw(enc2utf8(x)), "UTF-8", ..., as_html = as_html, 
            options = options)
    }
    else {
        con <- path_to_connection(x)
        if (inherits(con, "connection")) {
            read_xml.connection(con, encoding = encoding, ..., 
                as_html = as_html, base_url = x, options = options)
        }
        else {
            doc <- doc_parse_file(con, encoding = encoding, as_html = as_html, 
                options = options)
            xml_document(doc)
        }
    }
}
Browse[2]> ls()
[1] "as_html"  "encoding" "options"  "x"
Browse[2]> cat(x, file = "")
<?xml version="1.0" encoding="UTF-8"standalone="yes" ?>
<exprlist>
<equal_assign line1="1" col1="1" line2="1" col2="6" start="8" end="13">
<expr line1="1" col1="1" line2="1" col2="4" start="8" end="11">
<SYMBOL line1="1" col1="1" line2="1" col2="4" start="8" end="11">blah</SYMBOL>
</expr>
<EQ_ASSIGN line1="1" col1="5" line2="1" col2="5" start="12" end="12">=</EQ_ASSIGN>
<expr line1="1" col1="6" line2="1" col2="6" start="13" end="13">
<NUM_CONST line1="1" col1="6" line2="1" col2="6" start="13" end="13">1</NUM_CONST>
</equal_assign>
</expr>
</exprlist>

So for some reason, the xml corresponding to the lint-example looks like

<equal_assign   ...> <expr[1] ...> ... </expr[1]> ... <expr[2] ...> ... </equal_assign> </expr[2]>

@russHyde
Copy link
Collaborator

In the R=3.6.1 env that I'm using I have xmlparsedata=1.0.2 (the same as in my R 3.5.1 env).

I suspect there's a problem with how xmlparsedata::xml_parse_data is converting data frames into xml trees (though I don't know why it's a R 3.6 specific thing)

@jimhester
Copy link
Member Author

The R parser changed in regards to = and <- assignment in R 3.6 in the tokens it returns for = expressions, so that is the likely reason for the change.

@russHyde
Copy link
Collaborator

russHyde commented Aug 13, 2019

Thanks, I hadn't heard about that; I wonder why that would lead to the xml tree getting borked either side of the equal_assign in the above though. Will experiment further in xmlparsedata

erikcs added a commit to grf-labs/grf that referenced this issue Aug 14, 2019
A bug in lintr hinders the CI tests on R 3.6. Disable lintr for now and
revert this once r-lib/lintr#384 has been resolved by the lintr
developers.
@russHyde
Copy link
Collaborator

FYI I've ran all these tests using an updated xmlparsedata (following changes r-lib/xmlparsedata#6 to deal with the new equal_assign token in R-3.6's getParseData()) and using the current lintr master and they all pass locally in R-3.6.

@jimhester
Copy link
Member Author

Great! @russHyde could you add Remotes: r-lib/xmlparsedata to the DESCRIPTION file, so we can use this version on the CI servers?

@russHyde
Copy link
Collaborator

No problem. Would that ensure that the updated version of xmlparsedata would be used whenever people use the github-version of lintr to test their packages on CI?

@russHyde
Copy link
Collaborator

Also, would you welcome a separate PR where I add a couple of other releases of R to the CI checks for lintr (for 3.5, 3.6 and the dev version of R)? At present the travis config seems to install/test lintr against R-3.6 only

@jimhester
Copy link
Member Author

Would that ensure that the updated version of xmlparsedata would be used whenever people use the github-version of lintr to test their packages on CI?

yes

Also, would you welcome a separate PR where I add a couple of other releases of R to the CI checks for lintr

Sure, you can use usethis::use_tidy_ci() to do this for you if you want to use it.

@russHyde
Copy link
Collaborator

Fixed in #395

davidahirshberg pushed a commit to davidahirshberg/grf that referenced this issue Dec 6, 2019
A bug in lintr hinders the CI tests on R 3.6. Disable lintr for now and
revert this once r-lib/lintr#384 has been resolved by the lintr
developers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior help wanted ❤️ we'd love your help!
Projects
None yet
Development

No branches or pull requests

3 participants