nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MINIFI-290) Enable resource files to be loaded easily in unit tests
Date Fri, 19 May 2017 13:42:04 GMT

    [ https://issues.apache.org/jira/browse/MINIFI-290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16017402#comment-16017402
] 

ASF GitHub Bot commented on MINIFI-290:
---------------------------------------

Github user kevdoran commented on the issue:

    https://github.com/apache/nifi-minifi-cpp/pull/95
  
    @apiri @phrocker - update on this: 
    
    The approach of setting a hardcoded working directory when adding tests to ctest in the
CMakeLists.txt file proved to be too brittle. If you invoke the test executable directly,
it fails. After some exploration, I could not find a way to pass the resource location to
the Catch unit tests in a manner that I was satisfied with. Two alternatives I explored included
passing the location as an environment variable and passing it as a command line argument.
The environment variable approach was a no-go as it it would require us to setup the "test"
target in CMake using something other than enable_testing() and instead creating our own target,
which seemed undesirable at this time. The argument approach would require us to define our
own main for Catch unit tests, rather than using the `#define CATCH_CONFIG_MAIN`, and because
`#define CATCH_CONFIG_MAIN` does more than just inline a main function, I didn't want to do
that either in case assumptions made today break with other versions of Catch.hpp in the future.
    
    The benefits here seemed minor, and could not justify the extra complexity introduced
by the workarounds I explored (IMO), so I reverted the yaml config to being hardcoded inline
strings. For now, I recommend closing MINIFI-290 as "won't do" and merge this PR which has
a few other changes that I cam across while investigating this. Alternatively, we could move
MINIFI-290 back to "open" if we want to revisit this in the future... but I'm sure if the
need arises we would reopen it or make a new ticket.


> Enable resource files to be loaded easily in unit tests
> -------------------------------------------------------
>
>                 Key: MINIFI-290
>                 URL: https://issues.apache.org/jira/browse/MINIFI-290
>             Project: Apache NiFi MiNiFi
>          Issue Type: Improvement
>          Components: C++, Testing
>            Reporter: Kevin Doran
>            Assignee: Kevin Doran
>            Priority: Minor
>
> As part of MINIFI-275, unit test cases were introduced that rely on YAML configuration
input. Currently, the YAML is defined as string constants in the test cases (see [1]).
> During peer review of MINIFI-275, it was suggested by [~phrocker] to move the YAML inputs
to resource files and load them for the test. This ticket captures that improvement which
will cleanup the unit test code by making the YAML input easier to locate and maintain.
> As part of this, we need a clean way to set resource file locations in CMAKE so that
they are easily available in ctest test cases. As the `test` target which invokes ctest is
a builtin/standard CMAKE generated target, it is more limited in its configurability for items
such as command line arguments [2] and environment variables (SET (CTEST_ENVIRONMENT ...)
apparently does not work in CMakeLists.txt files, only when CMake is invoked via the CLI).
This needs some more experimenting / digging into with our specific version of CMAKE before
we decide on an approach for implementation.
> [1] https://github.com/apache/nifi-minifi-cpp/pull/85 
> [2] http://stackoverflow.com/a/16163137



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message