From b36cbe982512ad2933e8d2ead4dc9106ca49d40f Mon Sep 17 00:00:00 2001 From: rillig Date: Fri, 18 Oct 2019 21:42:20 +0000 Subject: [PATCH] pkgtools/R2pkg: refactorings, tests --- pkgtools/R2pkg/files/R2pkg.R | 74 ++++++++---------- pkgtools/R2pkg/files/R2pkg_test.R | 124 ++++++++++++++++++++---------- 2 files changed, 116 insertions(+), 82 deletions(-) diff --git a/pkgtools/R2pkg/files/R2pkg.R b/pkgtools/R2pkg/files/R2pkg.R index be92abd40e79..c00d0022c751 100644 --- a/pkgtools/R2pkg/files/R2pkg.R +++ b/pkgtools/R2pkg/files/R2pkg.R @@ -1,4 +1,4 @@ -# $NetBSD: R2pkg.R,v 1.12 2019/10/18 17:18:03 rillig Exp $ +# $NetBSD: R2pkg.R,v 1.13 2019/10/18 21:42:20 rillig Exp $ # # Copyright (c) 2014,2015,2016,2017,2018,2019 # Brook Milligan. All rights reserved. @@ -693,55 +693,50 @@ construct.line <- function(df,key,value) df } -element <- function(df,key,value,quiet=FALSE) +element <- function(mklines, varname, field, quiet=FALSE) { - key.index <- match(key,df$key,0) - if (key.index != 0 && df$key_value[key.index]) - result <- df[key.index,value] - else + i <- match(varname, mklines$key, 0) + if (i != 0 && mklines$key_value[i]) + return(mklines[i, field]) + + if (!quiet) { - result <- '???' - if (!quiet) - { - if (key.index == 0) - level.warning(key,' not found') - else - level.warning(key,' is not a key-value field') - } + if (i == 0) + level.warning(varname, ' not found') + else + level.warning(varname, ' is not a key-value field') } - result + '???' } -make.categories <- function(df) +make.categories <- function(mklines) { - # message('===> make.categories():') directory <- basename(dirname(getwd())) - categories <- unlist(element(df,'CATEGORIES','old_value')) - categories <- unlist(strsplit(categories,'[[:blank:]]+')) - categories <- c(directory,categories) - categories <- categories[ categories != 'R' ] + categories <- unlist(element(mklines, 'CATEGORIES', 'old_value')) + categories <- unlist(strsplit(categories, '[[:blank:]]+')) + categories <- c(directory, categories) + categories <- categories[categories != 'R'] if (directory != 'wip') - categories <- categories[ categories != 'wip' ] + categories <- categories[categories != 'wip'] categories <- categories[!duplicated(categories)] - categories <- paste(categories,collapse=' ') - categories + paste(categories, collapse = ' ') } -make.maintainer <- function(df) +make.maintainer <- function(mklines) { - old.maintainer <- element(df,'MAINTAINER','old_value') - new.maintainer <- element(df,'MAINTAINER','new_value') - ifelse(old.maintainer == '',new.maintainer,old.maintainer) + old.maintainer <- element(mklines, 'MAINTAINER', 'old_value') + new.maintainer <- element(mklines, 'MAINTAINER', 'new_value') + if (old.maintainer == '') new.maintainer else old.maintainer } -make.comment <- function(df) +make.comment <- function(mklines) { - old.comment <- element(df,'COMMENT','old_value') - new.comment <- element(df,'COMMENT','new_value') - comment <- old.comment - if (!weakly.equals(old.comment,new.comment)) - comment <- paste0(comment,'\t# [R2pkg] updated to: ',new.comment) - comment + old.comment <- element(mklines, 'COMMENT', 'old_value') + new.comment <- element(mklines, 'COMMENT', 'new_value') + if (weakly.equals(old.comment, new.comment)) + old.comment + else + paste0(old.comment, '\t# [R2pkg] updated to: ', new.comment) } make.new_license <- function(df,license) @@ -809,12 +804,8 @@ find.order <- function(df,key,field) value } -write.makefile <- function(lines) write(lines,'Makefile') - update.Makefile.with.metadata <- function(df,metadata) { - # message('===> update.Makefile.with.metadata():') - df$new_value <- NA df <- make.new_license(df,metadata$License) @@ -823,9 +814,6 @@ update.Makefile.with.metadata <- function(df,metadata) df$new_value[df$key == 'MAINTAINER'] <- arg.maintainer_email df$new_value[df$key == 'COMMENT'] <- one.line(metadata$Title) df$new_value[df$key == 'R_PKGVER'] <- one.line(metadata$Version) - - # str(df) - # print(df) df } @@ -1072,7 +1060,7 @@ update.Makefile <- function(metadata) df.buildlink3 <- make.df.buildlink3(df,BUILDLINK3.MK) df.makefile <- make.df.makefile(df,df.conflicts,df.depends,df.buildlink3) - write.makefile(df.makefile[,'new_line']) + write(df.makefile[, 'new_line'], 'Makefile') } create.Makefile <- function(metadata) diff --git a/pkgtools/R2pkg/files/R2pkg_test.R b/pkgtools/R2pkg/files/R2pkg_test.R index ffd451514c1c..52798242d3ba 100644 --- a/pkgtools/R2pkg/files/R2pkg_test.R +++ b/pkgtools/R2pkg/files/R2pkg_test.R @@ -1,4 +1,4 @@ -# $NetBSD: R2pkg_test.R,v 1.7 2019/10/18 17:18:03 rillig Exp $ +# $NetBSD: R2pkg_test.R,v 1.8 2019/10/18 21:42:20 rillig Exp $ # # Copyright (c) 2019 # Roland Illig. All rights reserved. @@ -38,7 +38,7 @@ mkcvsid = paste0('# $', 'NetBSD$') arg.recursive <- FALSE arg.update <- FALSE -package.dir <- file.path(Sys.getenv('PKGSRCDIR'), 'pkgtools', 'R2pkg') +package_dir <- file.path(Sys.getenv('PKGSRCDIR'), 'pkgtools', 'R2pkg') expect_printed <- function(obj, ...) { out <- '' @@ -369,13 +369,13 @@ test_that('depends', { }) test_that('depends.pkg', { - local_dir(package.dir) + local_dir(package_dir) expect_equal(depends.pkg('ellipsis'), '../../math/R-ellipsis') }) test_that('new.depends.pkg', { - local_dir(package.dir) + local_dir(package_dir) if (dir.exists('../../wip')) expect_equal(new.depends.pkg('C50'), '../../wip/R-C50') @@ -406,7 +406,7 @@ test_that('new.depends.pkg', { # }) test_that('buildlink3.file with matching version number', { - local_dir(package.dir) + local_dir(package_dir) dependency <- make.dependency('bitops(>=0.1)') bl3 <- buildlink3.file(dependency) @@ -417,7 +417,7 @@ test_that('buildlink3.file with matching version number', { # The version number of the dependency is not checked against # the resolved buildlink3 file. test_that('buildlink3.file with too high version number', { - local_dir(package.dir) + local_dir(package_dir) dependency <- make.dependency('bitops(>=1000.0)') bl3 <- buildlink3.file(dependency) @@ -426,7 +426,7 @@ test_that('buildlink3.file with too high version number', { }) test_that('buildlink3.line', { - local_dir(package.dir) + local_dir(package_dir) expect_equal( buildlink3.line(make.dependency('ellipsis')), @@ -469,14 +469,48 @@ test_that('use.languages with Rcpp as dependency', { # test_that('copy.description', { # }) -# test_that('write.Makefile', { -# }) +test_that('write.Makefile', { + tmpdir <- paste(tempdir(), 'category', 'pkgdir', sep = '/') + dir.create(tmpdir, recursive = TRUE) + local_dir(tmpdir) + metadata <- make.metadata(linesConnection( + 'Package: pkgname', + 'Version: 1.3', + 'Depends: ellipsis')) + + write.Makefile(metadata) + + expect_equal(readLines('Makefile'),c( + mkcvsid, + '', + 'R_PKGNAME=\tpkgname', + 'R_PKGVER=\t1.3', + 'CATEGORIES=\tcategory', + '', + 'MAINTAINER=\t', # FIXME + 'COMMENT=\tNA', # FIXME + 'LICENSE=\tNA', # FIXME + '', + 'USE_LANGUAGES=\t# none', + '', + '.include "../../math/R/Makefile.extension"', + '.include "../../mk/bsd.pkg.mk"' + )) +}) # test_that('construct.line', { # }) -# test_that('element', { -# }) +test_that('element', { + mklines <- read.Makefile.as.dataframe(linesConnection( + 'COMMENT=\tThe comment', + 'EMPTY=')) + + expect_equal(element(mklines, 'COMMENT', 'order'), 1) + expect_equal(element(mklines, 'COMMENT', 'old_value'), 'The comment') + expect_equal(element(mklines, 'UNKNOWN', 'order'), '???') # FIXME: should be a number + expect_equal(element(mklines, 'EMPTY', 'old_value'), '') +}) # test_that('make.categories', { # }) @@ -484,8 +518,16 @@ test_that('use.languages with Rcpp as dependency', { # test_that('make.maintainer', { # }) -# test_that('make.comment', { -# }) +test_that('make.comment', { + mklines <- read.Makefile.as.dataframe(linesConnection( + 'COMMENT=\tOld comment')) + + mklines$new_value[[1]] <- 'New comment' + expect_equal(make.comment(mklines), 'Old comment\t# [R2pkg] updated to: New comment') + + mklines$new_value[[1]] <- 'old Comment' + expect_equal(make.comment(mklines), 'Old comment') +}) # test_that('make.new_license', { # }) @@ -502,33 +544,40 @@ test_that('use.languages with Rcpp as dependency', { # test_that('make.r_pkgver', { # }) -# test_that('find.order', { -# }) +test_that('find.order', { + mklines <- read.Makefile.as.dataframe(linesConnection( + 'CATEGORIES=', + 'HOMEPAGE=', + 'USE_TOOLS+=', + '.include "other.mk"', + '# comment')) -# test_that('write.makefile', { -# }) + vars_order <- find.order(mklines, 'key_value', 'order') + include_order <- find.order(mklines, 'buildlink3.mk', 'order') + + expect_equal(mklines[, 'key_value'], c(TRUE, TRUE, TRUE, FALSE, FALSE)) + expect_equal(mklines[, 'buildlink3.mk'], c(FALSE, FALSE, FALSE, FALSE, FALSE)) + expect_equal(vars_order, c(1)) + expect_equal(include_order, NA_integer_) +}) test_that('update.Makefile.with.metadata', { + local_dir(package_dir) # to get a realistic category df <- read.Makefile.as.dataframe(linesConnection( - 'CATEGORIES=', - 'MAINTAINER=', - 'COMMENT=', - 'R_PKGVER=')) + 'CATEGORIES=\told categories', + 'MAINTAINER=\told_maintainer@example.org', + 'COMMENT=\told comment', + 'R_PKGVER=\t1.0')) metadata = list(Title = 'Package comment', Version = '19.3', License = 'license') updated <- update.Makefile.with.metadata(df, metadata) - expect_printed(updated, - ' line order category key_value key depends buildlink3.mk', - '1 CATEGORIES= 1 NA TRUE CATEGORIES FALSE FALSE', - '2 MAINTAINER= 2 NA TRUE MAINTAINER FALSE FALSE', - '3 COMMENT= 3 NA TRUE COMMENT FALSE FALSE', - '4 R_PKGVER= 4 NA TRUE R_PKGVER FALSE FALSE', - ' operator delimiter old_value old_todo new_value', - '1 = R2pkg', - '2 = ', - '3 = Package comment', - '4 = 19.3') + expect_printed(data.frame(key = updated$key, new_value = updated$new_value), + ' key new_value', + '1 CATEGORIES pkgtools', + '2 MAINTAINER ', # FIXME: Should not always be reset. + '3 COMMENT Package comment', + '4 R_PKGVER 19.3') }) # If the variable has been removed from the Makefile, it is not updated. @@ -621,13 +670,10 @@ test_that('update.Makefile', { '', '.include "../../mk/bsd.pkg.mk"'), 'Makefile.orig') - writeLines( - c( - 'Package: pkgname', - 'Version: 1.0', - 'Depends: dep1 dep2(>=2.0)'), - 'DESCRIPTION') - metadata <- make.metadata('DESCRIPTION') + metadata <- make.metadata(linesConnection( + 'Package: pkgname', + 'Version: 1.0', + 'Depends: dep1 dep2(>=2.0)')) expect_printed( as.data.frame(metadata), ' Package Version Title Description License Imports Depends',