Merge pull request #45 from siddharthab/unit

Unit tests for readProtoFiles2

NAMESPACE

@@ -78,7 +78,7 @@ exportMethods( "new", "[[", "[[<-", "$", "$<-", "show",
 
 	)
 
-export( "P", "readProtoFiles", "readProtoFiles2", "asMessage" )
+export( "P", "readProtoFiles", "readProtoFiles2", "resetDescriptorPool", "asMessage" )
 
 if( exists( ".DollarNames", asNamespace("utils") ) ) importFrom( utils, .DollarNames )
 S3method(.DollarNames, Message )

R/internals.R

@@ -101,6 +101,12 @@ readProtoFiles2 <- function(files,
 	invisible(NULL)
 }
 
+resetDescriptorPool <- function(){
+	.Call( "resetDescriptorPool_cpp", PACKAGE = "RProtoBuf" )
+	readProtoFiles2( protoPath=system.file( "proto", package="RProtoBuf", lib.loc=.RProtoBuf_libname) )
+	invisible(NULL)
+}
+
 getProtobufLibVersion <- function( format = FALSE ){
 	version <- .Call( "get_protobuf_library_version", PACKAGE = "RProtoBuf" )
 

R/zzz.R

@@ -3,11 +3,12 @@
     ##minversion <- packageDescription(pkgname, lib.loc=libname)$MinimumLibProtoVersion
     ##minversion <- as.integer( gsub( "[[:space:]]+", "", minversion ) )
     ##.Call( "check_libprotobuf_version", minversion, PACKAGE = "RProtoBuf" )
-    readProtoFiles( package=pkgname, lib.loc=libname )
+    readProtoFiles2( protoPath=system.file( "proto", package=pkgname, lib.loc=libname ) )
     attachDescriptorPool( pos = length(search()) )
     options("RProtoBuf.int64AsString" = FALSE)
     #if( exists( ".httpd.handlers.env", asNamespace( "tools" ) ) ){
     #    e <- tools:::.httpd.handlers.env
     #    e[["RProtoBuf"]] <- RProtoBuf.http.handler
     #}
+    .RProtoBuf_libname <<- libname
 }

inst/unitTests/data/cyclical/proj1/proto/a.proto

@@ -0,0 +1,3 @@
+syntax = "proto2";
+
+message A {}

inst/unitTests/data/cyclical/proj1/proto/c.proto

@@ -0,0 +1,7 @@
+syntax = "proto2";
+
+import "proto/b.proto";
+
+message C {
+  optional B b = 1;
+}

inst/unitTests/data/cyclical/proj2/proto/b.proto

@@ -0,0 +1,7 @@
+syntax = "proto2";
+
+import "proto/a.proto";
+
+message B {
+  optional A a = 1;
+}

inst/unitTests/data/recursive/subdir/subdir_message.proto

@@ -0,0 +1,9 @@
+syntax = "proto2";
+
+package protobuf_unittest_recursive_subdir;
+
+import "unittest_import.proto";
+
+message SubdirMessage {
+  optional protobuf_unittest_import.ImportMessage msg = 1;
+}

inst/unitTests/data/subdir/subdir_message.proto

@@ -0,0 +1,9 @@
+syntax = "proto2";
+
+package protobuf_unittest_subdir;
+
+import "unittest_import.proto";
+
+message SubdirMessage {
+  optional protobuf_unittest_import.ImportMessage msg = 1;
+}

inst/unitTests/data/unittest.proto

@@ -42,6 +42,9 @@ syntax = "proto2";
 // the extra file from the directory in which this file is
 import "unittest_import.proto";
 
+// To test if we can import a message in another directory.
+import "subdir/subdir_message.proto";
+
 // We don't put this in a package within proto2 because we need to make sure
 // that the generated code doesn't depend on being in the proto2 namespace.
 // In test_util.h we do "using namespace unittest = protobuf_unittest".

inst/unitTests/runit.import.R

@@ -14,11 +14,63 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
-test.import <- function() {
+.setUp <- function() {
+    resetDescriptorPool()
+}
+
+test.import.reset <- function() {
+    ## Verify that the common setup is resetting the descriptor pool.
+    ## Only the messages defined in the proto directory should be present.
+    checkTrue(all(grepl("^(rexp|tutorial|rprotobuf)\\.", objects("RProtoBuf:DescriptorPool"))))
+}
+
+test.import.error <- function() {
     ## Verify that we get a graceful error rather than a segfault.
     checkException(readProtoFiles(system.file("DESCRIPTION", package="RProtoBuf")))
 }
 
+test.import <- function() {
+    unit.test.data.dir <- system.file("unitTests", "data", package="RProtoBuf")
+    checkTrue(!exists("protobuf_unittest_import.ImportMessage", "RProtoBuf:DescriptorPool"))
+
+    readProtoFiles2(recursive = FALSE, protoPath=unit.test.data.dir)
+    checkTrue(exists("protobuf_unittest_import.ImportMessage", "RProtoBuf:DescriptorPool"))
+    # The following is imported by unittest.proto and hence, should have been imported implicitly.
+    checkTrue(exists("protobuf_unittest_subdir.SubdirMessage", "RProtoBuf:DescriptorPool"))
+    # The following is not imported by any of the top level files.
+    checkTrue(!exists("protobuf_unittest_recursive_subdir.SubdirMessage", "RProtoBuf:DescriptorPool"))
+}
+
+test.import.subdir <- function() {
+    unit.test.data.dir <- system.file("unitTests", "data", package="RProtoBuf")
+
+    # Any of these two will work.
+    readProtoFiles2(file="subdir/subdir_message.proto", protoPath=unit.test.data.dir)
+    readProtoFiles2(dir="subdir", protoPath=unit.test.data.dir)
+    checkTrue(exists("protobuf_unittest_subdir.SubdirMessage", "RProtoBuf:DescriptorPool"))
+
+    readProtoFiles2(dir="recursive", recursive=TRUE, protoPath=unit.test.data.dir)
+    checkTrue(exists("protobuf_unittest_recursive_subdir.SubdirMessage", "RProtoBuf:DescriptorPool"))
+}
+
+test.import.cyclical <- function() {
+    unit.test.cyclical.dir <- system.file("unitTests", "data", "cyclical", package="RProtoBuf")
+    proj1.dir <- file.path(unit.test.cyclical.dir, "proj1")
+    proj2.dir <- file.path(unit.test.cyclical.dir, "proj2")
+
+    # The following files should be loaded in order as C depends on B depends on A.
+
+    readProtoFiles2(file="proto/a.proto", protoPath=proj1.dir)
+    checkTrue(exists("A", "RProtoBuf:DescriptorPool"))
+    checkTrue(!exists("C", "RProtoBuf:DescriptorPool"))
+
+    readProtoFiles2(file="proto/b.proto", protoPath=proj2.dir)
+    checkTrue(exists("B", "RProtoBuf:DescriptorPool"))
+
+    readProtoFiles2(file="proto/c.proto", protoPath=proj1.dir)
+    checkTrue(exists("C", "RProtoBuf:DescriptorPool"))
+}
+
 test.assign.in.global <- function() {
     xx.undef <<- 3
     checkEquals(xx.undef, 3)

man/readProtoFiles.Rd

@@ -1,6 +1,7 @@
 \name{readProtoFiles}
 \alias{readProtoFiles}
 \alias{readProtoFiles2}
+\alias{resetDescriptorPool}
 \title{protocol buffer descriptor importer}
 \description{
   Imports proto files into the descriptor pool that 
@@ -10,6 +11,7 @@
 \usage{
   readProtoFiles(files, dir, package="RProtoBuf", pattern="\\\\.proto$", lib.loc=NULL)
   readProtoFiles2(files, dir=".", pattern="\\\\.proto$", recursive=FALSE, protoPath=getwd())
+  resetDescriptorPool()
 }
 \arguments{
   \item{files}{Proto files}
@@ -32,6 +34,8 @@
   interpreted relative to \code{protoPath}, so that there is consistency in
   future imports of the same files through import statements of other proto
   files.
+
+  \code{resetDescriptorPool} clears all imported proto definitions.
 }
 \value{\code{NULL}, invisibly.}
 \author{Romain Francois <francoisromain@free.fr>}

src/DescriptorPoolLookup.cpp

@@ -34,8 +34,9 @@ SEXP DescriptorPoolLookup::getElements() { return Rcpp::wrap(elements); }
 std::set<std::string> DescriptorPoolLookup::elements;
 RWarningErrorCollector DescriptorPoolLookup::error_collector;
 RSourceTree DescriptorPoolLookup::source_tree;
-GPB::compiler::Importer DescriptorPoolLookup::importer(&source_tree, &error_collector);
-GPB::DynamicMessageFactory DescriptorPoolLookup::message_factory(importer.pool());
+GPB::compiler::Importer* DescriptorPoolLookup::importer =
+    new GPB::compiler::Importer(&source_tree, &error_collector);
+GPB::DynamicMessageFactory DescriptorPoolLookup::message_factory(importer->pool());
 
 /**
  * Add descriptors from a proto file to the descriptor pool.
@@ -51,7 +52,7 @@ void DescriptorPoolLookup::importProtoFiles(SEXP files, SEXP dirs) {
     source_tree.addDirectories(dirs);
     int n = LENGTH(files);
     for (int j = 0; j < n; j++) {
-        const GPB::FileDescriptor* file_desc = importer.Import(CHAR(STRING_ELT(files, j)));
+        const GPB::FileDescriptor* file_desc = importer->Import(CHAR(STRING_ELT(files, j)));
         if (!file_desc) {
             std::string message = std::string("Could not load proto file '") + CHAR(STRING_ELT(files, j)) +
                 "'\n";
@@ -80,7 +81,18 @@ void DescriptorPoolLookup::importProtoFiles(SEXP files, SEXP dirs) {
     // source_tree.removeDirectories( dirs ) ;
 }
 
-const GPB::DescriptorPool* DescriptorPoolLookup::pool() { return importer.pool(); }
+/**
+ * Clears any persisted state for the descriptor pool.
+ */
+void DescriptorPoolLookup::reset() {
+    source_tree.removeAllDirectories();
+    elements.clear();
+    // TODO: Find out why deleting the old importer crashes the unit test run sometimes.
+    // delete importer;
+    importer = new GPB::compiler::Importer(&source_tree, &error_collector);
+}
+
+const GPB::DescriptorPool* DescriptorPoolLookup::pool() { return importer->pool(); }
 
 const GPB::DynamicMessageFactory* DescriptorPoolLookup::factory() { return &message_factory; }
 

src/DescriptorPoolLookup.h

@@ -19,6 +19,8 @@ class DescriptorPoolLookup {
 
     static void importProtoFiles(SEXP files, SEXP cwd);
 
+    static void reset();
+
     static const GPB::DescriptorPool* pool();
 
     static const GPB::DynamicMessageFactory* factory();
@@ -27,7 +29,7 @@ class DescriptorPoolLookup {
     static std::set<std::string> elements;
     static RWarningErrorCollector error_collector;
     static RSourceTree source_tree;
-    static GPB::compiler::Importer importer;
+    static GPB::compiler::Importer* importer;
     static GPB::DynamicMessageFactory message_factory;
 };
 

src/RSourceTree.cpp

@@ -48,5 +48,6 @@ void RSourceTree::removeDirectories(SEXP dirs) {
         directories.erase(std::string(CHAR(STRING_ELT(dirs, i))));
     }
 }
+void RSourceTree::removeAllDirectories() { directories.clear(); }
 
 }  // namespace rprotobuf

src/RSourceTree.h

@@ -10,6 +10,7 @@ class RSourceTree : public GPB::compiler::SourceTree {
     void addDirectories(SEXP dirs);
     void removeDirectory(const std::string& directory);
     void removeDirectories(SEXP dirs);
+    void removeAllDirectories();
 
    private:
     std::set<std::string> directories;

src/init.c

@@ -146,6 +146,7 @@ extern SEXP MethodDescriptor__output_type(SEXP);
 extern SEXP newProtocolBufferLookup(SEXP);
 extern SEXP newProtoMessage(SEXP);
 extern SEXP readProtoFiles_cpp(SEXP, SEXP);
+extern SEXP resetDescriptorPool_cpp();
 extern SEXP ServiceDescriptor__as_character(SEXP);
 extern SEXP ServiceDescriptor__as_list(SEXP);
 extern SEXP ServiceDescriptor__as_Message(SEXP);
@@ -301,6 +302,7 @@ static const R_CallMethodDef CallEntries[] = {
     {"newProtocolBufferLookup",                  (DL_FUNC) &newProtocolBufferLookup,                   1},
     {"newProtoMessage",                          (DL_FUNC) &newProtoMessage,                           1},
     {"readProtoFiles_cpp",                       (DL_FUNC) &readProtoFiles_cpp,                        2},
+    {"resetDescriptorPool_cpp",                  (DL_FUNC) &resetDescriptorPool_cpp,                   0},
     {"ServiceDescriptor__as_character",          (DL_FUNC) &ServiceDescriptor__as_character,           1},
     {"ServiceDescriptor__as_list",               (DL_FUNC) &ServiceDescriptor__as_list,                1},
     {"ServiceDescriptor__as_Message",            (DL_FUNC) &ServiceDescriptor__as_Message,             1},
@@ -341,3 +343,7 @@ void R_init_RProtoBuf(DllInfo *dll) {
     R_registerRoutines(dll, NULL, CallEntries, NULL, NULL);
     R_useDynamicSymbols(dll, FALSE);
 }
+
+void R_unload_RProtoBuf(DllInfo* dll) {
+    resetDescriptorPool_cpp();
+}

src/rprotobuf.cpp

@@ -50,6 +50,16 @@ RcppExport SEXP readProtoFiles_cpp(SEXP file, SEXP dirs) {
     END_RCPP
 }
 
+/**
+ * reset the descriptor pool, clearing all persisted state
+ */
+RcppExport SEXP resetDescriptorPool_cpp() {
+    BEGIN_RCPP
+    DescriptorPoolLookup::reset();
+    return R_NilValue;
+    END_RCPP
+}
+
 /**
  * get the descriptor associated with a message type
  *