Skip to content

Commit

Permalink
Fix R Unit Tests and Invalid Cursor State Occurrences (#100)
Browse files Browse the repository at this point in the history
* Fix code causing 'length(x) = 5 > 1' in coercion to 'logical(1)' warning.

* fix failing func execute in sql test due to differing environments

* add unittest for getServerVersion to detect bad cursor error on odbc

* add new R 4.2 to test workflow.

* adjust comment text and variable name

* Fix for spees generation codepath triggering cursor state invalid error

* Fix warning  longer object length is not a multiple of shorter object length

* support for special characters in connection string password

* updated version number of R package to 1.2.1 in DESCRIPTION

* update CRAN snapshot reference

Co-authored-by: Sean Leonard <seleonar@microsoft.com>
  • Loading branch information
seantleonard and Sean Leonard authored Jul 11, 2022
1 parent 27612c6 commit 054c248
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 11 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
strategy:
fail-fast: false
matrix:
r-version: ["3.5.2"]
r-version: ["3.5.2", "4.2"]

env:
# Define CI to skip some test case.
Expand Down Expand Up @@ -58,7 +58,7 @@ jobs:
#Updated odbc pkg is still compatible with R >= 3.2.0
cran::odbc
rcmdcheck

- uses: r-lib/actions/check-r-package@v2
with:
working-directory: ./R
Expand Down Expand Up @@ -91,4 +91,4 @@ jobs:
- name: Test with pytest
working-directory: ./Python/tests
run: |
pytest
pytest
4 changes: 2 additions & 2 deletions R/DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: sqlmlutils
Type: Package
Title: Wraps R code into executable SQL Server stored procedures
Version: 1.0.0
Version: 1.2.1
Author: Microsoft Corporation
Maintainer: Microsoft Corporation <msrpack@microsoft.com>
Depends:
Expand All @@ -13,7 +13,7 @@ Description: sqlmlutils is a package designed to help users interact with SQL Se
creating and running stored procedures, and managing packages on the database.
License: MIT + file LICENSE
Copyright: Copyright 2016 Microsoft Corporation
RoxygenNote: 7.1.0
RoxygenNote: 7.1.2
Encoding: UTF-8
Suggests: testthat (>= 2.0.0),
roxygen2
2 changes: 1 addition & 1 deletion R/R/executeInSQL.R
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ connectionInfo <- function(driver = "SQL Server", server = "localhost", database
}
else
{
authorization = sprintf("uid=%s;pwd=%s",uid,pwd)
authorization = sprintf("uid=%s;pwd={%s}",uid,pwd)
}
}

Expand Down
13 changes: 10 additions & 3 deletions R/R/sqlPackage.R
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,14 @@ sqlRemoteExecuteFun <- function(connection, FUN, ..., useRemoteFun = FALSE, asus
query <- paste0("EXECUTE AS USER = '", asuser, "';")
}

# Addresses Invalid Cursor State issue triggered by PRINT() statements
# and DIAG messages from SQL Server.
resultSet <- "WITH RESULT SETS((resultColumn varchar(MAX)))"
query <- paste0(query
,"\nEXEC sp_execute_external_script"
,"\n@language = N'", languageName, "'"
,"\n,@script = N'",script, "';"
,"\n,@script = N'",script, "'"
,"\n",resultSet, ";"
)

if (!is.null(asuser))
Expand Down Expand Up @@ -818,7 +822,7 @@ sqlCheckPackageManagementVersion <- function(connectionString)

version <- sqlPackageManagementVersion(connectionString)

if (is.null(version) || is.na(version) || length(version) == 0)
if (is.null(version) || any(is.na(version)) || length(version) == 0)
{
stop("Invalid SQL version is null or empty", call. = FALSE)
}
Expand Down Expand Up @@ -1169,8 +1173,11 @@ getDependentPackagesToInstall <- function(pkgs, availablePackages, installedPack

#
# Determine if package is available as a binary package
# Utilize first of possibly many contributor URLs present
# in character vector contribWinBinaryUrl
#
packageProperties <- availablePackages[availablePackages$Package == package & availablePackages$Repository == contribWinBinaryUrl, ]
contributorURL <- contribWinBinaryUrl[1]
packageProperties <- availablePackages[availablePackages$Package == package & availablePackages$Repository == contributorURL, ]

#
# When only a source package is available, add LinkingTo dependencies
Expand Down
2 changes: 1 addition & 1 deletion R/tests/testthat/helper-Setup.R
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ testthatDir <- getwd()
R_Root <- file.path(testthatDir, "../..")
scriptDirectory <- file.path(testthatDir, "scripts")

options(repos = c(CRAN="https://cran.microsoft.com", CRANextra = "https://mran.microsoft.com/snapshot/2019-02-01"))
options(repos = c(CRAN="https://cran.microsoft.com/snapshot/2022-07-06"))
cat("INFO: repos = ", getOption("repos"), sep="\n")

# Compute context specifications
Expand Down
3 changes: 2 additions & 1 deletion R/tests/testthat/test.executeInSqlTests.R
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ test_that("Returning a function object",
return(func2)
}

expect_equal(executeFunctionInSQL(connection, func=func1), func2)
# Result of executeFunctionInSQL() will have different environment than func2.
expect_equal(executeFunctionInSQL(connection, func=func1), func2, check.environment=FALSE)
})

test_that("Calling an object in the environment",
Expand Down
6 changes: 6 additions & 0 deletions R/tests/testthat/test.sqlPackage.unit.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ test_that("Package management ExtLib", {
versionClass <- sqlmlutils:::sqlCheckPackageManagementVersion(connectionString = helper_getSetting("connectionStringDBO"))
expect_equal(versionClass, "ExtLib")
})

test_that("GetServerVersion() Returns Server Version of R Successfully",{
rversion <- sqlmlutils:::getserverVersion(connectionString = cnnstr, languageName = "R")
# rversion value truncated, so R may be >= 3.5 (3.5.3) or >= 4.2
expect_gte(as.double(rversion[['rversion']]), 3.5)
})

0 comments on commit 054c248

Please sign in to comment.