[GitHub] [logging-log4cxx] stephen-webb commented on issue #18: Conditionally exclude tests that require the external 'zip' or 'gzip'

[GitBox <gi...@apache.org>]

stephen-webb commented on issue #18: Conditionally exclude tests that require the external 'zip' or 'gzip'
URL: https://github.com/apache/logging-log4cxx/pull/18#issuecomment-586885978
 
 
   I have not been through all the tests that use sed in as much detail as
   timebasedrollingtestcase. It takes quite a lot of time to understand what
   each test is doing.
   The two rolling test cases failed on Windows as gzip and zip are not
   available. I spent today getting to an understanding of the problem. Still
   not fully aware of why the non-empty output directory caused failures.
   
   I will not be able to do more until next weeekend now.
   
   On Mon, Feb 17, 2020, 7:53 PM Thorsten Schöning <no...@github.com>
   wrote:
   
   > *@ams-tschoening* requested changes on this pull request.
   >
   > In this commit you implemented an approach EXCLUDING individual tests if
   > ZIP and GZIP are NOT available, while in the past you have implemented
   > INCLUDING a complete test file only
   > <https://github.com/apache/logging-log4cxx/blob/9ae6c6108597426dc463f248844c48d5592b20cb/src/test/cpp/CMakeLists.txt#L33>
   > when SED IS available.
   >
   > if(sedProgram)
   >     list(APPEND ALL_LOG4CXX_TESTS minimumtestcase patternlayouttest)
   >
   > Shouldn't we use one of both approaches only? I think I like the latter
   > more, EXCLUDING individual things on request, because that allows people
   > not using CMAKE currently, like me ;-), to either provide the necessary
   > binaries and things will work by default or EXCLUDE individual tests
   > because of missing binaries. By default we shouldn't EXCLUDE in my opinion,
   > as is currently the case, because SED, ZIP and GZIP can easily be provided
   > on Windows using UnixTools or MSYS2 embedded into GIT for Windows etc.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/logging-log4cxx/pull/18?email_source=notifications&email_token=ANLOSDRRXURO5P3S6QPAMNTRDJGCDA5CNFSM4KWLP2VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCVXJRAI#pullrequestreview-359569537>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ANLOSDRX6JCGNU2PJW2MKZDRDJGCDANCNFSM4KWLP2VA>
   > .
   >
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services