[Libreoffice-commits] core.git: sal/qa

[Chris Sherlock]

 sal/qa/osl/pipe/osl_Pipe.cxx |   47 +++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

New commits:
commit e0f990b96f901be00ba5b7dc6a671e6162c914cf
Author: Chris Sherlock <chris.sherlock79 at gmail.com>
Date:   Tue Jul 18 06:46:38 2017 +1000

    osl: fix Pipe noacquire/acquire tests
    
    There is an incomplete test of ::osl::Pipe::Pipe(oslPipe, __sal_NoAcquire),
    but the comments show that the tester wasn't able to figure out a way to
    test the noacquire variant of this constructor. However, they seem to
    have not just given up but copied and pasted the acquire and no acquire
    tests - the error codes for the acquire test talk about non-acquisition
    and don't actually see if the handle is the same or not.
    
    I have fixed this test. To test if the noacquire is working or not then
    you merely have to:
    
    1. create a new Pipe instance
    2. create a new Pipe non-acquired instance copy of the first instance
    3. delete the non-acquired instance copy
    4. try to send a single character to the original Pipe instance, which
       should return an error (negative return result) because at this
       point there is no valid pipe to operate on as the pipe should have
       been destroyed when you deleted the copy.
    
    If the send() succeeds, then this is a test failure.
    
    For the acquire test, it didn't actually test if the handles are the
    same - I have corrected this now.
    
    Change-Id: If868746233d3a222cc6f9e7cd0d453e70ef6c7cc
    Reviewed-on: https://gerrit.libreoffice.org/40102
    Reviewed-by: Chris Sherlock <chris.sherlock79 at gmail.com>
    Tested-by: Chris Sherlock <chris.sherlock79 at gmail.com>

diff --git a/sal/qa/osl/pipe/osl_Pipe.cxx b/sal/qa/osl/pipe/osl_Pipe.cxx
index d6a165785795..fcf227eb2541 100644
--- a/sal/qa/osl/pipe/osl_Pipe.cxx
+++ b/sal/qa/osl/pipe/osl_Pipe.cxx
@@ -117,7 +117,7 @@ namespace osl_Pipe
     class ctors : public CppUnit::TestFixture
     {
     public:
-        bool bRes, bRes1;
+        bool bRes, bRes1, bRes2;
 
         void ctors_none( )
             {
@@ -165,29 +165,27 @@ namespace osl_Pipe
                                         bRes );
             }
 
-        /**  tester comment:
-
-        When test the following two constructors, don't know how to test the
-        acquire and no acquire action. possible plans:
-        1.release one handle and check the other( did not success since the
-        other still exist and valid. )
-        2. release one handle twice to see getLastError( )(the getLastError
-        always returns invalidError(LINUX)).
-        */
-
+        /* Note: DO NOT DO THIS - I have very deliberately caused send to FAIL, *on purpose* as this is the
+           only sane way to test noacquire. This is a terrible misuse of no-acquire, but in this case is
+           needed only so we can test to make sure no-acquire is working!
+         */
         void ctors_no_acquire( )
             {
                 /// create a pipe.
-                ::osl::Pipe aPipe( test::uniquePipeName(aTestPipeName), osl_Pipe_CREATE );
-                osl_acquirePipe(aPipe.getHandle());
+                ::osl::Pipe* pPipe = new ::osl::Pipe( test::uniquePipeName(aTestPipeName), osl_Pipe_CREATE );
                 /// constructs a pipe reference without acquiring the handle.
-                ::osl::Pipe aNoAcquirePipe( aPipe.getHandle( ), SAL_NO_ACQUIRE );
+                ::osl::Pipe* pNoAcquirePipe = new ::osl::Pipe( pPipe->getHandle( ), SAL_NO_ACQUIRE );
 
-                bRes = aNoAcquirePipe.is( );
-                ///aPipe.clear( );
-                ///bRes1 = aNoAcquirePipe.is( );
+                StreamPipe aStreamPipe(pPipe->getHandle());
+                delete pNoAcquirePipe;
+                int nRet = aStreamPipe.send("a", 1);
 
-                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test constructor with no acquire of handle, only validation test, do not know how to test no acquire.",
+                if (nRet >= 0)
+                    bRes = false;
+                else
+                    bRes = true;
+
+                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test constructor with no acquire of handle, deleted nonacquired pipe but could still send on original pipe!.",
                                         bRes );
             }
 
@@ -195,17 +193,20 @@ namespace osl_Pipe
             {
                 /// create a base pipe.
                 ::osl::Pipe aPipe( test::uniquePipeName(aTestPipeName), osl_Pipe_CREATE );
-                /// constructs two pipes without acquiring the handle on the base pipe.
+                /// constructs two pipes, the second acquires the first pipe's handle.
                 ::osl::Pipe aAcquirePipe( aPipe.getHandle( ) );
                 ::osl::Pipe aAcquirePipe1( nullptr );
 
-                bRes = aAcquirePipe.is( );
-                bRes1 = aAcquirePipe1.is( );
+                bRes = aAcquirePipe.is();
+                bRes1 = aAcquirePipe1.is();
+                bRes2 = aPipe == aAcquirePipe;
 
-                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test constructor with no acquire of handle.only validation test, do not know how to test no acquire.",
+                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test constructor with acquire of handle, original pipe does not exist.",
                                         bRes );
-                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test constructor with no acquire of handle.only validation test, do not know how to test no acquire.",
+                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test constructor with acquire of handle, copied pipe does not exist",
                                         !bRes1 );
+
+                CPPUNIT_ASSERT_MESSAGE( "#test comment#: test pipes should have same handle", bRes2);
             }
 
         CPPUNIT_TEST_SUITE( ctors );