Thrift
  1. Thrift
  2. THRIFT-847

Test Framework harmonization across all languages

    Details

      Description

      Today each Language supported by Thrift, have its own unit test, all are using the same Thrift IDL's located at the test directory. But the behavior of these tests seems to be different from language to language... this makes it difficult to do tests and bug fixing across different Languages. e.g.

      • C++ Test and JavaScript Test Server written in Java have different responses for the same services
      • C# and Java Test Server have different responses for testException as C++

      I propose the following steps:

      • identify the language with the reference implementation (well defined return values for all test cases)
      • update the ThriftTest.thrift with details about the required return values that have to be implemented
      • update test implementations and move language tests into their appropriate library directory (THRIFT-35)
      • a public test server that supports multiple protocols and transports could be another enhancement for testing purposes

      I'm ready to help preparing patches, just tell me what you need!

      1. v1-WORK_IN_PROGRESS-unified_tests.tar.gz
        6 kB
        Christian Lavoie
      2. build.xml
        3 kB
        Roger Meier
      3. test.sh
        4 kB
        Roger Meier
      4. THRIFT-847_add__make_cross__build_target.patch
        0.2 kB
        Roger Meier
      5. 0001-THRIFT-847-Test-Framework-harmonization-across-all-l.patch
        2 kB
        Chamila Dilshan Wijayarathna
      6. 0001-THRIFT-847-Test-Framework-harmonization-across-all-l.patch
        3 kB
        Chamila Dilshan Wijayarathna
      7. 0001-THRIFT-847-Test-Framework-harmonization-across-all-l.patch
        8 kB
        Chamila Dilshan Wijayarathna
      8. 0001-THRIFT-847-Test-Framework-harmonization-across-all-l.patch
        15 kB
        Chamila Dilshan Wijayarathna
      9. 0001-THRIFT-847-Test-Framework-harmonization-across-all-l.patch
        27 kB
        Chamila Dilshan Wijayarathna
      10. 0001-THRIFT-847-Test-Framework-harmonization-across-all-l.patch
        8 kB
        Roger Meier

        Issue Links

          Activity

          Hide
          Bryan Duxbury added a comment -

          This is a really cool idea, but I don't think this is going to make it into TRUNK before the 0.4 branch gets cut.

          Show
          Bryan Duxbury added a comment - This is a really cool idea, but I don't think this is going to make it into TRUNK before the 0.4 branch gets cut.
          Hide
          Christian Lavoie added a comment -

          So, here's a very rough draft of unfinished cross language tests. To run, cd into a working thrift tree, extract the tar, it'll create a crosstest/ subdir with a few shell scripts:

          $ ls
          SimpleTest.cpp		SimpleTest.thrift	java-build.sh
          SimpleTest.hs		cpp-build.sh		java-run.sh
          SimpleTest.java		hs-build.sh
          

          To build the tests:

          $ ./java-build.sh && ./cpp-build.sh && ./hs-build.sh 
          Note: Some input files use unchecked or unsafe operations.
          Note: Recompile with -Xlint:unchecked for details.
          [ 1 of 14] Compiling Thrift.Transport ( ../lib/hs/src/Thrift/Transport.hs, build-hs/Thrift/Transport.o )
          [ 2 of 14] Compiling Thrift.Transport.Handle ( ../lib/hs/src/Thrift/Transport/Handle.hs, build-hs/Thrift/Transport/Handle.o )
          [ 3 of 14] Compiling Thrift.Protocol  ( ../lib/hs/src/Thrift/Protocol.hs, build-hs/Thrift/Protocol.o )
          [ 4 of 14] Compiling Thrift.Protocol.Binary ( ../lib/hs/src/Thrift/Protocol/Binary.hs, build-hs/Thrift/Protocol/Binary.o )
          [ 5 of 14] Compiling Thrift           ( ../lib/hs/src/Thrift.hs, build-hs/Thrift.o )
          [ 6 of 14] Compiling Thrift.Server    ( ../lib/hs/src/Thrift/Server.hs, build-hs/Thrift/Server.o )
          [ 7 of 14] Compiling SimpleTest_Types ( gen-hs/SimpleTest_Types.hs, build-hs/SimpleTest_Types.o )
          [ 8 of 14] Compiling Base_Iface       ( gen-hs/Base_Iface.hs, build-hs/Base_Iface.o )
          [ 9 of 14] Compiling Simple_Iface     ( gen-hs/Simple_Iface.hs, build-hs/Simple_Iface.o )
          [10 of 14] Compiling Base             ( gen-hs/Base.hs, build-hs/Base.o )
          [11 of 14] Compiling Simple           ( gen-hs/Simple.hs, build-hs/Simple.o )
          [12 of 14] Compiling Base_Client      ( gen-hs/Base_Client.hs, build-hs/Base_Client.o )
          [13 of 14] Compiling Simple_Client    ( gen-hs/Simple_Client.hs, build-hs/Simple_Client.o )
          [14 of 14] Compiling Main             ( SimpleTest.hs, build-hs/Main.o )
          Linking hs-test ...
          

          And to run the tests in both mode (each language is run against itself, but through a TCP socket):

          $ ./cpp-simple-test both && ./java-run.sh both && ./hs-test both
          Running in both client-and-server mode
          Running byte tests
          arg: both
          Starting server on port 9090 ...
          Running byte numeric tests
          Running i16 numeric tests
          Running i32 numeric tests
          Running i64 numeric tests
          Running double numeric tests
          SUCCESS
          Runing in both client and server mode
          Testing Simple.addBytes...
          

          You can also run each pair of executables with one in server and one in client mode:

          $ ./java-run.sh server & sleep 5 && ./cpp-simple-test client && ./hs-test client && sleep 1 && kill $(jobs -p)
          [1] 12018
          arg: server
          Starting server on port 9090 ...
          Running in client-only mode
          Running byte tests
          Runing in client-only mode
          Testing Simple.addBytes...
          

          On my Mac machine, I can get all pairs (and all three tests in both modes) to run with 0.5.0, but NOT with HEAD; the cpp client seems to be broken since dreiss' facebook patches; just guessing here, but the error message is telling:

          $ ./hs-test server & sleep 3 && ./cpp-simple-test client
          [1] 22775
          Runing in server-only mode
          Running in client-only mode
          Running byte tests
          terminate called after throwing an instance of 'apache::thrift::transport::TTransportException'
            what():  Base TTransport cannot write.
          Abort trap
          

          So, here's my plan, and my RFC:

          1. Finish the simplest possible tests, and implement a handful of clients (I suspect I'll get perl, haskell, c++ and java done) and a handful of servers (haskell, c++ and java; most likely)
          2. Get this hooked to the autotools build system and make check
          3. Define a set of base tests: you aren't a proper thrift bindings unless you pass those
          4. Define different set of advanced tests: you match specific features exactly as the reference languages do (== java, cpp, I think). In particular, I introduced a bug where Haskell consider byte to be unsigned, and java/cpp don't. Under this definition, Haskell would fail the advanced tests that check for numerical type implementation details
          5. Create some sort of score card for each language – to see how closely they match the reference implementation behaviour

          Please comment on the approach, I'd like some feedback before I spend too much time implementing tests nobody will like.

          Show
          Christian Lavoie added a comment - So, here's a very rough draft of unfinished cross language tests. To run, cd into a working thrift tree, extract the tar, it'll create a crosstest/ subdir with a few shell scripts: $ ls SimpleTest.cpp SimpleTest.thrift java-build.sh SimpleTest.hs cpp-build.sh java-run.sh SimpleTest.java hs-build.sh To build the tests: $ ./java-build.sh && ./cpp-build.sh && ./hs-build.sh Note: Some input files use unchecked or unsafe operations. Note: Recompile with -Xlint:unchecked for details. [ 1 of 14] Compiling Thrift.Transport ( ../lib/hs/src/Thrift/Transport.hs, build-hs/Thrift/Transport.o ) [ 2 of 14] Compiling Thrift.Transport.Handle ( ../lib/hs/src/Thrift/Transport/Handle.hs, build-hs/Thrift/Transport/Handle.o ) [ 3 of 14] Compiling Thrift.Protocol ( ../lib/hs/src/Thrift/Protocol.hs, build-hs/Thrift/Protocol.o ) [ 4 of 14] Compiling Thrift.Protocol.Binary ( ../lib/hs/src/Thrift/Protocol/Binary.hs, build-hs/Thrift/Protocol/Binary.o ) [ 5 of 14] Compiling Thrift ( ../lib/hs/src/Thrift.hs, build-hs/Thrift.o ) [ 6 of 14] Compiling Thrift.Server ( ../lib/hs/src/Thrift/Server.hs, build-hs/Thrift/Server.o ) [ 7 of 14] Compiling SimpleTest_Types ( gen-hs/SimpleTest_Types.hs, build-hs/SimpleTest_Types.o ) [ 8 of 14] Compiling Base_Iface ( gen-hs/Base_Iface.hs, build-hs/Base_Iface.o ) [ 9 of 14] Compiling Simple_Iface ( gen-hs/Simple_Iface.hs, build-hs/Simple_Iface.o ) [10 of 14] Compiling Base ( gen-hs/Base.hs, build-hs/Base.o ) [11 of 14] Compiling Simple ( gen-hs/Simple.hs, build-hs/Simple.o ) [12 of 14] Compiling Base_Client ( gen-hs/Base_Client.hs, build-hs/Base_Client.o ) [13 of 14] Compiling Simple_Client ( gen-hs/Simple_Client.hs, build-hs/Simple_Client.o ) [14 of 14] Compiling Main ( SimpleTest.hs, build-hs/Main.o ) Linking hs-test ... And to run the tests in both mode (each language is run against itself, but through a TCP socket): $ ./cpp-simple-test both && ./java-run.sh both && ./hs-test both Running in both client-and-server mode Running byte tests arg: both Starting server on port 9090 ... Running byte numeric tests Running i16 numeric tests Running i32 numeric tests Running i64 numeric tests Running double numeric tests SUCCESS Runing in both client and server mode Testing Simple.addBytes... You can also run each pair of executables with one in server and one in client mode: $ ./java-run.sh server & sleep 5 && ./cpp-simple-test client && ./hs-test client && sleep 1 && kill $(jobs -p) [1] 12018 arg: server Starting server on port 9090 ... Running in client-only mode Running byte tests Runing in client-only mode Testing Simple.addBytes... On my Mac machine, I can get all pairs (and all three tests in both modes) to run with 0.5.0, but NOT with HEAD; the cpp client seems to be broken since dreiss' facebook patches; just guessing here, but the error message is telling: $ ./hs-test server & sleep 3 && ./cpp-simple-test client [1] 22775 Runing in server-only mode Running in client-only mode Running byte tests terminate called after throwing an instance of 'apache::thrift::transport::TTransportException' what(): Base TTransport cannot write. Abort trap So, here's my plan, and my RFC: Finish the simplest possible tests, and implement a handful of clients (I suspect I'll get perl, haskell, c++ and java done) and a handful of servers (haskell, c++ and java; most likely) Get this hooked to the autotools build system and make check Define a set of base tests: you aren't a proper thrift bindings unless you pass those Define different set of advanced tests: you match specific features exactly as the reference languages do (== java, cpp, I think). In particular, I introduced a bug where Haskell consider byte to be unsigned, and java/cpp don't. Under this definition, Haskell would fail the advanced tests that check for numerical type implementation details Create some sort of score card for each language – to see how closely they match the reference implementation behaviour Please comment on the approach, I'd like some feedback before I spend too much time implementing tests nobody will like.
          Hide
          Roger Meier added a comment -

          Thanks Christian for your inputs.

          I had the following things in mind:

          1. identify the language with the reference implementation (well defined return values for all test cases) based on ThriftTest.thrift, probably Java (lib/java/test/org/apache/thrift/server/ServerTestBase.java) or cpp (test/cpp/src/TestClient.cpp test/cpp/src/TestServer.cpp)
          2. update the ThriftTest.thrift with details about the required return values that have to be implemented
          3. update test implementations
          4. add some wrapper scripts to automate the cross language tests (as Christian did)
          5. move language tests into their appropriate library directory (THRIFT-35)
          6. a public test server that supports multiple protocols and transports could be another enhancement for testing purposes

          Today neary every language has already Test Server and Test Clients based on ThriftTest.thrift , we just have to modify them a bit so that they have exactly the same behavior. I think it is not worth to add another SimpleTest.thrift, the target should be extending ThriftTest.thrift.

          Show
          Roger Meier added a comment - Thanks Christian for your inputs. I had the following things in mind: identify the language with the reference implementation (well defined return values for all test cases) based on ThriftTest.thrift, probably Java (lib/java/test/org/apache/thrift/server/ServerTestBase.java) or cpp (test/cpp/src/TestClient.cpp test/cpp/src/TestServer.cpp) update the ThriftTest.thrift with details about the required return values that have to be implemented update test implementations add some wrapper scripts to automate the cross language tests (as Christian did) move language tests into their appropriate library directory ( THRIFT-35 ) a public test server that supports multiple protocols and transports could be another enhancement for testing purposes Today neary every language has already Test Server and Test Clients based on ThriftTest.thrift , we just have to modify them a bit so that they have exactly the same behavior. I think it is not worth to add another SimpleTest.thrift, the target should be extending ThriftTest.thrift.
          Hide
          Roger Meier added a comment -

          THRIFT-960 provides Java TestServer and Client

          Show
          Roger Meier added a comment - THRIFT-960 provides Java TestServer and Client
          Hide
          Roger Meier added a comment -

          What do you think abou something like that build.xml?
          using the regular test server and clients and do interaction tests across languages?
          this build.xml just does c++ tests, but can be extended if we harmonize the parameters for the test suites across languages.

          Show
          Roger Meier added a comment - What do you think abou something like that build.xml ? using the regular test server and clients and do interaction tests across languages? this build.xml just does c++ tests, but can be extended if we harmonize the parameters for the test suites across languages.
          Hide
          Roger Meier added a comment -

          I did some rework and have a first version for a test/test.sh which can be executed independently on the ci server.

          further work is adding much more tests and integrate test suites into autoconf build procedure

          What do you think about that?

          Show
          Roger Meier added a comment - I did some rework and have a first version for a test/ test.sh which can be executed independently on the ci server. further work is adding much more tests and integrate test suites into autoconf build procedure What do you think about that?
          Hide
          Henrique Mendonça added a comment -

          Awesome Roger, thanks a lot!
          As you said, perhaps we could create a separate make target for that, or use 'check', so it would handle to dependencies and read the configuration (e.g. test all, and only, configured languages...)
          but that's a lot more work, and this is already working perfectly
          Cheers!

          Show
          Henrique Mendonça added a comment - Awesome Roger, thanks a lot! As you said, perhaps we could create a separate make target for that, or use 'check', so it would handle to dependencies and read the configuration (e.g. test all, and only, configured languages...) but that's a lot more work, and this is already working perfectly Cheers!
          Hide
          Hudson added a comment -

          Integrated in Thrift #378 (See https://builds.apache.org/job/Thrift/378/)
          THRIFT-847 Test Framework harmonization across all languages
          add test.sh as a first approach

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1229359
          Files :

          • /thrift/trunk/.gitignore
          • /thrift/trunk/test/test.sh
          Show
          Hudson added a comment - Integrated in Thrift #378 (See https://builds.apache.org/job/Thrift/378/ ) THRIFT-847 Test Framework harmonization across all languages add test.sh as a first approach roger : http://svn.apache.org/viewvc/?view=rev&rev=1229359 Files : /thrift/trunk/.gitignore /thrift/trunk/test/test.sh
          Hide
          Roger Meier added a comment -

          committed test/test.sh and added to the CI job
          see https://builds.apache.org/view/S-Z/view/Thrift/job/Thrift/lastSuccessfulBuild/artifact/thrift/test/test.log

          however, timeout which is usually a part of coreutils is not installed on the build servers
          I will request that now.

          Show
          Roger Meier added a comment - committed test/test.sh and added to the CI job see https://builds.apache.org/view/S-Z/view/Thrift/job/Thrift/lastSuccessfulBuild/artifact/thrift/test/test.log however, timeout which is usually a part of coreutils is not installed on the build servers I will request that now.
          Hide
          Hudson added a comment -

          Integrated in Thrift #383 (See https://builds.apache.org/job/Thrift/383/)
          THRIFT-847 Test Framework harmonization across all languages
          perl fixes:

          • generate Makfile for perl tests
          • add inc path to test

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1229745
          Files :

          • /thrift/trunk/.gitignore
          • /thrift/trunk/configure.ac
          • /thrift/trunk/test/perl/Makefile
          • /thrift/trunk/test/perl/Makefile.am
          • /thrift/trunk/test/test.sh
          Show
          Hudson added a comment - Integrated in Thrift #383 (See https://builds.apache.org/job/Thrift/383/ ) THRIFT-847 Test Framework harmonization across all languages perl fixes: generate Makfile for perl tests add inc path to test roger : http://svn.apache.org/viewvc/?view=rev&rev=1229745 Files : /thrift/trunk/.gitignore /thrift/trunk/configure.ac /thrift/trunk/test/perl/Makefile /thrift/trunk/test/perl/Makefile.am /thrift/trunk/test/test.sh
          Hide
          Hudson added a comment -

          Integrated in Thrift #384 (See https://builds.apache.org/job/Thrift/384/)
          THRIFT-847 Test Framework harmonization across all languages
          perl fixes:

          • include test/perl if perl is enabled

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1229764
          Files :

          • /thrift/trunk/test/Makefile.am
          Show
          Hudson added a comment - Integrated in Thrift #384 (See https://builds.apache.org/job/Thrift/384/ ) THRIFT-847 Test Framework harmonization across all languages perl fixes: include test/perl if perl is enabled roger : http://svn.apache.org/viewvc/?view=rev&rev=1229764 Files : /thrift/trunk/test/Makefile.am
          Hide
          Roger Meier added a comment -

          see:
          https://builds.apache.org/view/S-Z/view/Thrift/job/Thrift/lastSuccessfulBuild/artifact/thrift/test/test.log

          yes, we have some issues... but I'm sure we will fix them! patches are welcome!
          -r

          Show
          Roger Meier added a comment - see: https://builds.apache.org/view/S-Z/view/Thrift/job/Thrift/lastSuccessfulBuild/artifact/thrift/test/test.log yes, we have some issues... but I'm sure we will fix them! patches are welcome! -r
          Hide
          Hudson added a comment -

          Integrated in Thrift #393 (See https://builds.apache.org/job/Thrift/393/)
          THRIFT-847 Test Framework harmonization across all languages
          add php TestClient to the testsuite

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1234292
          Files :

          • /thrift/trunk/.gitignore
          • /thrift/trunk/configure.ac
          • /thrift/trunk/test/Makefile.am
          • /thrift/trunk/test/php/Makefile
          • /thrift/trunk/test/php/Makefile.am
          • /thrift/trunk/test/test.sh
          Show
          Hudson added a comment - Integrated in Thrift #393 (See https://builds.apache.org/job/Thrift/393/ ) THRIFT-847 Test Framework harmonization across all languages add php TestClient to the testsuite roger : http://svn.apache.org/viewvc/?view=rev&rev=1234292 Files : /thrift/trunk/.gitignore /thrift/trunk/configure.ac /thrift/trunk/test/Makefile.am /thrift/trunk/test/php/Makefile /thrift/trunk/test/php/Makefile.am /thrift/trunk/test/test.sh
          Hide
          Hudson added a comment -

          Integrated in Thrift #580 (See https://builds.apache.org/job/Thrift/580/)
          THRIFT-847 Test Framework harmonization across all languages
          add NODE_PATH
          remove Unix Domain Socket used by tests (Revision 1404889)
          THRIFT-1745 Python JSON protocol
          TJSONProtocol.py: Frederic Delbos

          THRIFT-847 Test Framework harmonization across all languages
          Integration into py lib and test suite (Revision 1404838)

          Result = FAILURE
          roger : http://svn.apache.org/viewvc/?view=rev&rev=1404889
          Files :

          • /thrift/trunk/test/nodejs/Makefile.am
          • /thrift/trunk/test/test.sh

          roger : http://svn.apache.org/viewvc/?view=rev&rev=1404838
          Files :

          • /thrift/trunk/lib/py/src/protocol/TJSONProtocol.py
          • /thrift/trunk/lib/py/src/protocol/_init_.py
          • /thrift/trunk/test/py/RunClientServer.py
          • /thrift/trunk/test/py/TestClient.py
          • /thrift/trunk/test/py/TestServer.py
          • /thrift/trunk/test/test.sh
          Show
          Hudson added a comment - Integrated in Thrift #580 (See https://builds.apache.org/job/Thrift/580/ ) THRIFT-847 Test Framework harmonization across all languages add NODE_PATH remove Unix Domain Socket used by tests (Revision 1404889) THRIFT-1745 Python JSON protocol TJSONProtocol.py: Frederic Delbos THRIFT-847 Test Framework harmonization across all languages Integration into py lib and test suite (Revision 1404838) Result = FAILURE roger : http://svn.apache.org/viewvc/?view=rev&rev=1404889 Files : /thrift/trunk/test/nodejs/Makefile.am /thrift/trunk/test/test.sh roger : http://svn.apache.org/viewvc/?view=rev&rev=1404838 Files : /thrift/trunk/lib/py/src/protocol/TJSONProtocol.py /thrift/trunk/lib/py/src/protocol/_ init _.py /thrift/trunk/test/py/RunClientServer.py /thrift/trunk/test/py/TestClient.py /thrift/trunk/test/py/TestServer.py /thrift/trunk/test/test.sh
          Hide
          Hudson added a comment -

          Integrated in Thrift #615 (See https://builds.apache.org/job/Thrift/615/)
          THRIFT-847 Test Framework harmonization across all languages (Revision f42ce2a8f49cf09e695974e6cd3c434b8dda61ab)

          Result = FAILURE
          roger : https://git-wip-us.apache.org/repos/asf?p=thrift.git&a=commit&h=f42ce2a8f49cf09e695974e6cd3c434b8dda61ab
          Files :

          • lib/java/build.xml
          • test/test.sh
          • lib/java/test/org/apache/thrift/test/TestServer.java
          • lib/java/test/org/apache/thrift/test/TestClient.java
          Show
          Hudson added a comment - Integrated in Thrift #615 (See https://builds.apache.org/job/Thrift/615/ ) THRIFT-847 Test Framework harmonization across all languages (Revision f42ce2a8f49cf09e695974e6cd3c434b8dda61ab) Result = FAILURE roger : https://git-wip-us.apache.org/repos/asf?p=thrift.git&a=commit&h=f42ce2a8f49cf09e695974e6cd3c434b8dda61ab Files : lib/java/build.xml test/test.sh lib/java/test/org/apache/thrift/test/TestServer.java lib/java/test/org/apache/thrift/test/TestClient.java
          Hide
          Roger Meier added a comment -

          THRIFT-847_add_make_cross_build_target.patch adds a *make cross* target to run the cross language test suite

          Show
          Roger Meier added a comment - THRIFT-847_add_ make_cross _build_target.patch adds a * make cross * target to run the cross language test suite
          Hide
          Roger Meier added a comment -

          committed make cross
          ;-r

          Show
          Roger Meier added a comment - committed make cross ;-r
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1056 (See https://builds.apache.org/job/Thrift/1056/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 7e10329f7982f2602d6dbdcb2b45e843b85170c4)

          • Makefile.am
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1056 (See https://builds.apache.org/job/Thrift/1056/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 7e10329f7982f2602d6dbdcb2b45e843b85170c4) Makefile.am
          Hide
          Jake Farrell added a comment -

          Roger Meier objections to closing this specific issue since we have the Test Suite component and this will be something that we will always be working on and subtasks/depends will always be part of Test Suite

          Show
          Jake Farrell added a comment - Roger Meier objections to closing this specific issue since we have the Test Suite component and this will be something that we will always be working on and subtasks/depends will always be part of Test Suite
          Hide
          Roger Meier added a comment -

          README.md committed.

          Show
          Roger Meier added a comment - README.md committed.
          Hide
          Roger Meier added a comment -

          The goal of this GSoC task is:
          • Cover any possible language, protocol, transport, type, etc. vice versa(Just a few are integrated right now)
          • Port cross language test suite to non linux systems
          • Improve test and functionality reporting towards features list for the web site
          • Fix documentation
          • Quality in general
          • Help to make it perfect… I’m sure there is enough to do

          as soon as we have candidates we can setup a new ticket or sub tasks for this.

          Show
          Roger Meier added a comment - The goal of this GSoC task is: • Cover any possible language, protocol, transport, type, etc. vice versa(Just a few are integrated right now) • Port cross language test suite to non linux systems • Improve test and functionality reporting towards features list for the web site • Fix documentation • Quality in general • Help to make it perfect… I’m sure there is enough to do as soon as we have candidates we can setup a new ticket or sub tasks for this.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1065 (See https://builds.apache.org/job/Thrift/1065/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev f85fdd7a3af3255ea2464118930c10f582caf035)

          • test/README.md
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1065 (See https://builds.apache.org/job/Thrift/1065/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev f85fdd7a3af3255ea2464118930c10f582caf035) test/README.md
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1121 (See https://builds.apache.org/job/Thrift/1121/)
          THRIFT-847: Test Framework harmonization across all languages (roger: rev 5829a2c64a47bb122a4c7e6ddf93acd6b41dfd7d)

          • test/test.sh
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1121 (See https://builds.apache.org/job/Thrift/1121/ ) THRIFT-847 : Test Framework harmonization across all languages (roger: rev 5829a2c64a47bb122a4c7e6ddf93acd6b41dfd7d) test/test.sh
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Thrift #1135 (See https://builds.apache.org/job/Thrift/1135/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 188024e4b2a647d140d3861ca0b4f28c7d45ac1e)

          • test/test.sh
          Show
          Hudson added a comment - FAILURE: Integrated in Thrift #1135 (See https://builds.apache.org/job/Thrift/1135/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 188024e4b2a647d140d3861ca0b4f28c7d45ac1e) test/test.sh
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1144 (See https://builds.apache.org/job/Thrift/1144/)
          THRIFT-847 Test Framework harmonization across all languages (henrique: rev d17f1c98e64e6392a981308516eca5f489c27c57)

          • test/test.sh
          • .travis.yml
          • lib/java/build.xml
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1144 (See https://builds.apache.org/job/Thrift/1144/ ) THRIFT-847 Test Framework harmonization across all languages (henrique: rev d17f1c98e64e6392a981308516eca5f489c27c57) test/test.sh .travis.yml lib/java/build.xml
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Thrift #1146 (See https://builds.apache.org/job/Thrift/1146/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 4edac7ff085e4bd28096fef3cf8234e4991544ff)

          • test/README.md
          • test/test.sh
          Show
          Hudson added a comment - FAILURE: Integrated in Thrift #1146 (See https://builds.apache.org/job/Thrift/1146/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 4edac7ff085e4bd28096fef3cf8234e4991544ff) test/README.md test/test.sh
          Hide
          Chamila Dilshan Wijayarathna added a comment -

          Attached patch adds "nodejs server - java client" and "java server - nodejs client" tests to cross language test suite.

          Show
          Chamila Dilshan Wijayarathna added a comment - Attached patch adds "nodejs server - java client" and "java server - nodejs client" tests to cross language test suite.
          Hide
          Chamila Dilshan Wijayarathna added a comment -

          Added
          --port arg (=9090) Port number to listen
          to nodejs server at lib/nodejs/test/server.js and added
          --host arg (=localhost) Host to connect
          --port arg (=9090) Port number to connect
          to nodejs client at lib/nodejs/test/client.js

          Show
          Chamila Dilshan Wijayarathna added a comment - Added --port arg (=9090) Port number to listen to nodejs server at lib/nodejs/test/server.js and added --host arg (=localhost) Host to connect --port arg (=9090) Port number to connect to nodejs client at lib/nodejs/test/client.js
          Hide
          Roger Meier added a comment -

          Chamila,
          please take care on coding style you see within source and trailing whitespaces.
          I had to fix these, before I committed your patch.

          Thank you!
          -roger

          Show
          Roger Meier added a comment - Chamila, please take care on coding style you see within source and trailing whitespaces. I had to fix these, before I committed your patch. Thank you! -roger
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1157 (See https://builds.apache.org/job/Thrift/1157/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 5c6ad2427c67023a67e873d2e389838394053272)

          • test/test.sh
            THRIFT-847 Test Framework harmonization across all languages (roger: rev f8c1c989f2116ef8cecb85dcf16657c04fc27435)
          • lib/nodejs/test/server.js
          • lib/nodejs/test/client.js
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1157 (See https://builds.apache.org/job/Thrift/1157/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 5c6ad2427c67023a67e873d2e389838394053272) test/test.sh THRIFT-847 Test Framework harmonization across all languages (roger: rev f8c1c989f2116ef8cecb85dcf16657c04fc27435) lib/nodejs/test/server.js lib/nodejs/test/client.js
          Hide
          Chamila Dilshan Wijayarathna added a comment -

          py-py, py-java, java-py, cpp-py, py-cpp, nodejs-py, py-nodejs tests added with
          py_protocols="binary compact json accel"
          py_transports="buffered"
          py_sockets="ip",
          test.sh refactored so that new protocols, transports and sockets can be easily added.

          Show
          Chamila Dilshan Wijayarathna added a comment - py-py, py-java, java-py, cpp-py, py-cpp, nodejs-py, py-nodejs tests added with py_protocols="binary compact json accel" py_transports="buffered" py_sockets="ip", test.sh refactored so that new protocols, transports and sockets can be easily added.
          Hide
          Roger Meier added a comment -

          Thanks Chamila, committed!

          failed tests / nr of test cases
          30 / 137
          49 / 161
          54 / 174 => now

          Show
          Roger Meier added a comment - Thanks Chamila, committed! failed tests / nr of test cases 30 / 137 49 / 161 54 / 174 => now
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1159 (See https://builds.apache.org/job/Thrift/1159/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 72268b78a39fb035a13bbe552774099d44b1c0de)

          • test/test.sh
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1159 (See https://builds.apache.org/job/Thrift/1159/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 72268b78a39fb035a13bbe552774099d44b1c0de) test/test.sh
          Hide
          Chamila Dilshan Wijayarathna added a comment - - edited

          Added 'ssl' tests for python tests.
          Added ' transport arg (=buffered) transport: buffered, framed, http' to test/py/TestServer.py and test/py/TestClient.py and removed 'framed' arguement.
          Changed test/py/RunClientServer.py to match above changes.
          Added tests to compact protocol in python cases.
          Added tests to test BinaryAccelarated protocol with Binary Protocol.

          Please check if changes done in TestServer.py and TestClient.py breaks any other fatures, I checked dependencies mentions by Roger Meier, (test/py/Makefile.am, test/py/RunClientServer.py, test/test.sh) and did necessary changes.

          112/272 Successful.

          Show
          Chamila Dilshan Wijayarathna added a comment - - edited Added 'ssl' tests for python tests. Added ' transport arg (=buffered) transport: buffered, framed, http' to test/py/TestServer.py and test/py/TestClient.py and removed 'framed' arguement. Changed test/py/RunClientServer.py to match above changes. Added tests to compact protocol in python cases. Added tests to test BinaryAccelarated protocol with Binary Protocol. Please check if changes done in TestServer.py and TestClient.py breaks any other fatures, I checked dependencies mentions by Roger Meier , (test/py/Makefile.am, test/py/RunClientServer.py, test/test.sh) and did necessary changes. 112/272 Successful.
          Hide
          Roger Meier added a comment -

          Thanks Chamila, committed.

          I changed --proto to --protocol within Python. This will simplify the test suite in the future.
          ... there is lot of redundant code within test.sh and a refactoring might become necessary soon.

          Show
          Roger Meier added a comment - Thanks Chamila, committed. I changed --proto to --protocol within Python. This will simplify the test suite in the future. ... there is lot of redundant code within test.sh and a refactoring might become necessary soon.
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Thrift #1169 (See https://builds.apache.org/job/Thrift/1169/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 76150722af2751768411573b9fbbed163f4f55db)

          • test/py/TestServer.py
          • test/py/RunClientServer.py
          • test/py/TestClient.py
          • test/test.sh
          Show
          Hudson added a comment - FAILURE: Integrated in Thrift #1169 (See https://builds.apache.org/job/Thrift/1169/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 76150722af2751768411573b9fbbed163f4f55db) test/py/TestServer.py test/py/RunClientServer.py test/py/TestClient.py test/test.sh
          Hide
          Chamila Dilshan Wijayarathna added a comment - - edited

          Added cross tests for ruby with ruby, cpp, java, node and python.
          ruby_protocols="binary compact json accel"
          ruby_transports="buffered framed"
          ruby_sockets="ip"
          Created new TestServer.rb and TestClient.rb for this purpose.

          In MultiExceptionTest and ExceptionTest in Ruby server, it throws "Thrift::TransportException: end of file reached" for all clients. I have no idea what causes this. We need to identify if this is and issue in Server implementation or problem of thrift.
          Ruby Client is working properly with java and node, but stuck in some tests against cpp and py without moving ahead for next tests. This is also need to be checked, if this is a issue of client I implemented or problem inside thrift. With java and node, ruby client works properly, but tests still fails due to incompatibility of nil values (Server sending 0 where client expecting 'nil'). I'll work on this, but I need some advice on how to proceed.

          Roger Meier
          Following are the things I think important to be done next.
          1. Adding 'ssl' tests for ruby. Does ruby support ssl? If so how can I add them to client and server?
          2. in test/rb/integration/ there are simple_server.rb, simple_client.rb, buffered_client.rb, accelerated_buffered_server.rb and accelerated_buffered_client.rb. TestServer.rb and TestClient.rb I added, contains every functionality those files have. So we can remove those files and replace them with TestServer.rb and TestClient.rb. What are the places which uses above files?
          3. Java and few other tests missing 'testStringMap'. What is the reason for that. If there is no huge reason, we need to add it also.

          Show
          Chamila Dilshan Wijayarathna added a comment - - edited Added cross tests for ruby with ruby, cpp, java, node and python. ruby_protocols="binary compact json accel" ruby_transports="buffered framed" ruby_sockets="ip" Created new TestServer.rb and TestClient.rb for this purpose. In MultiExceptionTest and ExceptionTest in Ruby server, it throws "Thrift::TransportException: end of file reached" for all clients. I have no idea what causes this. We need to identify if this is and issue in Server implementation or problem of thrift. Ruby Client is working properly with java and node, but stuck in some tests against cpp and py without moving ahead for next tests. This is also need to be checked, if this is a issue of client I implemented or problem inside thrift. With java and node, ruby client works properly, but tests still fails due to incompatibility of nil values (Server sending 0 where client expecting 'nil'). I'll work on this, but I need some advice on how to proceed. Roger Meier Following are the things I think important to be done next. 1. Adding 'ssl' tests for ruby. Does ruby support ssl? If so how can I add them to client and server? 2. in test/rb/integration/ there are simple_server.rb, simple_client.rb, buffered_client.rb, accelerated_buffered_server.rb and accelerated_buffered_client.rb. TestServer.rb and TestClient.rb I added, contains every functionality those files have. So we can remove those files and replace them with TestServer.rb and TestClient.rb. What are the places which uses above files? 3. Java and few other tests missing 'testStringMap'. What is the reason for that. If there is no huge reason, we need to add it also.
          Hide
          Roger Meier added a comment -

          Thanks Chamila!

          To your questions:
          1. yes, please add this should be supported
          2. I have removed them
          3. I guess testStringMap was introduced later and was probably not implemented on every language

          great work!
          -roger

          Show
          Roger Meier added a comment - Thanks Chamila! To your questions: 1. yes, please add this should be supported 2. I have removed them 3. I guess testStringMap was introduced later and was probably not implemented on every language great work! -roger
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1176 (See https://builds.apache.org/job/Thrift/1176/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev a3570ac36716d0313e2c1c6143cfffc5ddae8fec)

          • test/rb/integration/accelerated_buffered_client.rb
          • test/rb/integration/simple_client.rb
          • test/rb/integration/accelerated_buffered_server.rb
          • test/rb/integration/test_simple_handler.rb
          • test/test.sh
          • test/rb/integration/TestClient.rb
          • test/rb/integration/simple_server.rb
          • test/rb/integration/buffered_client.rb
          • test/rb/integration/TestServer.rb
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1176 (See https://builds.apache.org/job/Thrift/1176/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev a3570ac36716d0313e2c1c6143cfffc5ddae8fec) test/rb/integration/accelerated_buffered_client.rb test/rb/integration/simple_client.rb test/rb/integration/accelerated_buffered_server.rb test/rb/integration/test_simple_handler.rb test/test.sh test/rb/integration/TestClient.rb test/rb/integration/simple_server.rb test/rb/integration/buffered_client.rb test/rb/integration/TestServer.rb
          Hide
          Chamila Dilshan Wijayarathna added a comment -

          Roger Meier ,
          Is there any files other than test.sh which used those removed file? If so we need to do changes there also.

          Show
          Chamila Dilshan Wijayarathna added a comment - Roger Meier , Is there any files other than test.sh which used those removed file? If so we need to do changes there also.
          Hide
          Roger Meier added a comment -

          initial version of test.py and tests.json as future replacement of test.sh

          main goals I had in mind:

          • seperate file to define the tests for server and clients
          • cross platform (=> become able to run it on windows)
          • scripting language

          missing things:

          • reporting page
          • log files
          • add languages
          • run on windows
          Show
          Roger Meier added a comment - initial version of test.py and tests.json as future replacement of test.sh main goals I had in mind: seperate file to define the tests for server and clients cross platform (=> become able to run it on windows) scripting language missing things: reporting page log files add languages run on windows
          Hide
          Roger Meier added a comment -

          Chamila Dilshan Wijayarathna no other usage of the removed files, done.

          Show
          Roger Meier added a comment - Chamila Dilshan Wijayarathna no other usage of the removed files, done.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1178 (See https://builds.apache.org/job/Thrift/1178/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 40cc23269da6880d83dce17ff05a545d16fd787c)

          • test/test.py
          • test/tests.json
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1178 (See https://builds.apache.org/job/Thrift/1178/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 40cc23269da6880d83dce17ff05a545d16fd787c) test/test.py test/tests.json
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Thrift #1253 (See https://builds.apache.org/job/Thrift/1253/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev bea273484d63e5c14481754da9ff786b12835471)

          • test/test.sh
          Show
          Hudson added a comment - FAILURE: Integrated in Thrift #1253 (See https://builds.apache.org/job/Thrift/1253/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev bea273484d63e5c14481754da9ff786b12835471) test/test.sh
          Hide
          ASF GitHub Bot added a comment -

          GitHub user cdwijayarathna opened a pull request:

          https://github.com/apache/thrift/pull/190

          THRIFT-847 Test Framework harmonization across all languages

          Haskell tests added to test.py and test.sh

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/cdwijayarathna/thrift THRIFT-847

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/thrift/pull/190.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #190


          commit 4f96910047ecf7c18a40b74191cd7b501508fc55
          Author: cdwijayarathna <cdwijayarathna@gmail.com>
          Date: 2014-08-15T16:48:30Z

          THRIFT-847 Test Framework harmonization across all languages


          Show
          ASF GitHub Bot added a comment - GitHub user cdwijayarathna opened a pull request: https://github.com/apache/thrift/pull/190 THRIFT-847 Test Framework harmonization across all languages Haskell tests added to test.py and test.sh You can merge this pull request into a Git repository by running: $ git pull https://github.com/cdwijayarathna/thrift THRIFT-847 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/190.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #190 commit 4f96910047ecf7c18a40b74191cd7b501508fc55 Author: cdwijayarathna <cdwijayarathna@gmail.com> Date: 2014-08-15T16:48:30Z THRIFT-847 Test Framework harmonization across all languages
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52356290

          @bufferoverflow Added changes suggested by @zilberstein in https://github.com/cdwijayarathna/thrift/commit/33fe8f6da989181060d4c502a27426b7d57ff046 , should I create new PR with a single commit?

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52356290 @bufferoverflow Added changes suggested by @zilberstein in https://github.com/cdwijayarathna/thrift/commit/33fe8f6da989181060d4c502a27426b7d57ff046 , should I create new PR with a single commit?
          Hide
          ASF GitHub Bot added a comment -

          Github user zilberstein commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52356546

          @cdwijayarathna, you can squash your commits and then force push to the branch

          Show
          ASF GitHub Bot added a comment - Github user zilberstein commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52356546 @cdwijayarathna, you can squash your commits and then force push to the branch
          Hide
          ASF GitHub Bot added a comment -

          Github user bufferoverflow commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52357551

          The changes within .travis.yml and contrib/installDependencies.sh are not required.
          you can do this:

              diff --git a/lib/hs/Thrift.cabal b/lib/hs/Thrift.cabal
              index f847663..eb60bb5 100755
              --- a/lib/hs/Thrift.cabal
              +++ b/lib/hs/Thrift.cabal
              @@ -36,7 +36,7 @@ Library
                 Hs-Source-Dirs:
                   src
                 Build-Depends:
              -    base >= 4, base < 5, containers, network, ghc-prim, attoparsec, binary, bytestring >= 0.10, hashable, HTTP, text,
              +    base >= 4, base < 5, containers, network, ghc-prim, attoparsec, binary, bytestring >= 0.10, hashable, HTTP, text,
                 Exposed-Modules:
                   Thrift,
                   Thrift.Arbitraries
              

          test/tests.json is committed

          Show
          ASF GitHub Bot added a comment - Github user bufferoverflow commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52357551 The changes within .travis.yml and contrib/installDependencies.sh are not required. you can do this: diff --git a/lib/hs/Thrift.cabal b/lib/hs/Thrift.cabal index f847663..eb60bb5 100755 --- a/lib/hs/Thrift.cabal +++ b/lib/hs/Thrift.cabal @@ -36,7 +36,7 @@ Library Hs-Source-Dirs: src Build-Depends: - base >= 4, base < 5, containers, network, ghc-prim, attoparsec, binary, bytestring >= 0.10, hashable, HTTP, text, + base >= 4, base < 5, containers, network, ghc-prim, attoparsec, binary, bytestring >= 0.10, hashable, HTTP, text, Exposed-Modules: Thrift, Thrift.Arbitraries test/tests.json is committed
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52358462

          @bufferoverflow Added suggested changes and committed. Is there anything else I need to do for this?

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52358462 @bufferoverflow Added suggested changes and committed. Is there anything else I need to do for this?
          Hide
          ASF GitHub Bot added a comment -

          Github user bufferoverflow commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52358999

          please fix all the white space issues
          e.g contrib/installDependencies.sh and many other areas had no changes

          Show
          ASF GitHub Bot added a comment - Github user bufferoverflow commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52358999 please fix all the white space issues e.g contrib/installDependencies.sh and many other areas had no changes
          Hide
          ASF GitHub Bot added a comment -

          Github user bufferoverflow commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52359230

          You also might start using --port=$

          {THRIFT_TEST_PORT}
          Show
          ASF GitHub Bot added a comment - Github user bufferoverflow commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52359230 You also might start using --port=$ {THRIFT_TEST_PORT}
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52359274

          I removed trailing white spaces in files I edited, do I need to undo them?

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52359274 I removed trailing white spaces in files I edited, do I need to undo them?
          Hide
          ASF GitHub Bot added a comment -

          Github user zilberstein commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16316474

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          — End diff –

          This pattern match is not exhaustive. Also add the `-n` case here

          Show
          ASF GitHub Bot added a comment - Github user zilberstein commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16316474 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of — End diff – This pattern match is not exhaustive. Also add the `-n` case here
          Hide
          ASF GitHub Bot added a comment -

          Github user bufferoverflow commented on the pull request:

          https://github.com/apache/thrift/pull/190#issuecomment-52360323

          sorry, I did not recognized that. Perfect!
          committed

          Show
          ASF GitHub Bot added a comment - Github user bufferoverflow commented on the pull request: https://github.com/apache/thrift/pull/190#issuecomment-52360323 sorry, I did not recognized that. Perfect! committed
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16316696

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          — End diff –

          Sorry, I didn't understand what you suggested,

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16316696 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of — End diff – Sorry, I didn't understand what you suggested,
          Hide
          ASF GitHub Bot added a comment -

          Github user zilberstein commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16317049

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          — End diff –

          1) The pattern match case on line 204/217 should be integrated into the `case` statement here.
          2) You match a couple of strings, but a non-exhaustive pattern match error will be thrown if none of these strings match. You should add a default case with the wildcard operator (`_`) that evaluated to `Nothing` so that if you enter an unsupported flag it will print the help message

          Show
          ASF GitHub Bot added a comment - Github user zilberstein commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16317049 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of — End diff – 1) The pattern match case on line 204/217 should be integrated into the `case` statement here. 2) You match a couple of strings, but a non-exhaustive pattern match error will be thrown if none of these strings match. You should add a default case with the wildcard operator (`_`) that evaluated to `Nothing` so that if you enter an unsupported flag it will print the help message
          Hide
          ASF GitHub Bot added a comment -

          Github user zilberstein commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16317067

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          + "--port" : arg : _ -> parseFlags flags opts

          { port = read arg }

          + "--domain-socket" : arg : _ -> parseFlags flags opts

          { domainSocket = read arg }

          + "--host" : arg : _ -> parseFlags flags opts

          { host = arg }

          + "--transport" : arg : _ -> parseFlags flags opts

          { transport = arg }

          + "--protocol" : arg : _ -> parseFlags flags opts

          { protocol = getProtocol arg }

          + "--h" : _ -> Nothing
          + "--help" : _ -> Nothing
          + "--ssl" : _ -> parseFlags flags opts

          { ssl = True }

          + "--processor-events" : _ -> parseFlags flags opts
          — End diff –

          ie, after this line
          _ -> Nothing

          Show
          ASF GitHub Bot added a comment - Github user zilberstein commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16317067 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of + "--port" : arg : _ -> parseFlags flags opts { port = read arg } + "--domain-socket" : arg : _ -> parseFlags flags opts { domainSocket = read arg } + "--host" : arg : _ -> parseFlags flags opts { host = arg } + "--transport" : arg : _ -> parseFlags flags opts { transport = arg } + "--protocol" : arg : _ -> parseFlags flags opts { protocol = getProtocol arg } + "--h" : _ -> Nothing + "--help" : _ -> Nothing + "--ssl" : _ -> parseFlags flags opts { ssl = True } + "--processor-events" : _ -> parseFlags flags opts — End diff – ie, after this line _ -> Nothing
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Thrift #1259 (See https://builds.apache.org/job/Thrift/1259/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev dace6937a7c1f53c45c005f0d5d6e2851b656da6)

          • test/tests.json
            THRIFT-847 Test Framework harmonization across all languages (roger: rev d92179129a5674784da7248f09bd41d32bb762bc)
          • test/test.sh
          • lib/hs/Thrift.cabal
          • contrib/installDependencies.sh
          • test/hs/TestClient.hs
          • test/hs/TestServer.hs
          Show
          Hudson added a comment - FAILURE: Integrated in Thrift #1259 (See https://builds.apache.org/job/Thrift/1259/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev dace6937a7c1f53c45c005f0d5d6e2851b656da6) test/tests.json THRIFT-847 Test Framework harmonization across all languages (roger: rev d92179129a5674784da7248f09bd41d32bb762bc) test/test.sh lib/hs/Thrift.cabal contrib/installDependencies.sh test/hs/TestClient.hs test/hs/TestServer.hs
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16324539

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          — End diff –

          How can I integrate two cases?

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16324539 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of — End diff – How can I integrate two cases?
          Hide
          ASF GitHub Bot added a comment -

          Github user zilberstein commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16325863

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          — End diff –

          see https://gist.github.com/zilberstein/52a4e04788810e0f1ce4

          Show
          ASF GitHub Bot added a comment - Github user zilberstein commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16325863 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of — End diff – see https://gist.github.com/zilberstein/52a4e04788810e0f1ce4
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna closed the pull request at:

          https://github.com/apache/thrift/pull/190

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna closed the pull request at: https://github.com/apache/thrift/pull/190
          Hide
          ASF GitHub Bot added a comment -

          GitHub user cdwijayarathna opened a pull request:

          https://github.com/apache/thrift/pull/193

          THRIFT-847 Test Framework harmonization across all languages

          Edits suggested by @zilberstein added to Haskell tests.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/cdwijayarathna/thrift THRIFT-847

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/thrift/pull/193.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #193


          commit 82e0f9d988f8d96b410c1363f15389ef47b40def
          Author: cdwijayarathna <cdwijayarathna@gmail.com>
          Date: 2014-08-16T18:06:07Z

          THRIFT-847 Test Framework harmonization across all languages

          THRIFT-847 Test Framework harmonization across all languages


          Show
          ASF GitHub Bot added a comment - GitHub user cdwijayarathna opened a pull request: https://github.com/apache/thrift/pull/193 THRIFT-847 Test Framework harmonization across all languages Edits suggested by @zilberstein added to Haskell tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cdwijayarathna/thrift THRIFT-847 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/193.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #193 commit 82e0f9d988f8d96b410c1363f15389ef47b40def Author: cdwijayarathna <cdwijayarathna@gmail.com> Date: 2014-08-16T18:06:07Z THRIFT-847 Test Framework harmonization across all languages THRIFT-847 Test Framework harmonization across all languages
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna commented on a diff in the pull request:

          https://github.com/apache/thrift/pull/190#discussion_r16325997

          — Diff: test/hs/TestClient.hs —
          @@ -197,24 +198,24 @@ main = do
          Binary -> runClient $ BinaryProtocol handle
          Compact -> runClient $ CompactProtocol handle
          JSON -> runClient $ JSONProtocol handle

          • replicateM_ testLoops client
            + replicateM_ testLoops client
            putStrLn "COMPLETED SUCCESSFULLY"

          parseFlags :: [String] -> Options -> Maybe Options
          +parseFlags (flag : flags) opts = do
          + let pieces = splitOn "=" flag
          + case pieces of
          — End diff –

          https://github.com/apache/thrift/pull/193

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna commented on a diff in the pull request: https://github.com/apache/thrift/pull/190#discussion_r16325997 — Diff: test/hs/TestClient.hs — @@ -197,24 +198,24 @@ main = do Binary -> runClient $ BinaryProtocol handle Compact -> runClient $ CompactProtocol handle JSON -> runClient $ JSONProtocol handle replicateM_ testLoops client + replicateM_ testLoops client putStrLn "COMPLETED SUCCESSFULLY" parseFlags :: [String] -> Options -> Maybe Options +parseFlags (flag : flags) opts = do + let pieces = splitOn "=" flag + case pieces of — End diff – https://github.com/apache/thrift/pull/193
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Thrift #1261 (See https://builds.apache.org/job/Thrift/1261/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev 7191bc99fe3de1027f7fab21232d5bc6ed1d5db5)

          • test/hs/TestClient.hs
          • test/hs/TestServer.hs
          Show
          Hudson added a comment - FAILURE: Integrated in Thrift #1261 (See https://builds.apache.org/job/Thrift/1261/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev 7191bc99fe3de1027f7fab21232d5bc6ed1d5db5) test/hs/TestClient.hs test/hs/TestServer.hs
          Hide
          ASF GitHub Bot added a comment -

          Github user cdwijayarathna closed the pull request at:

          https://github.com/apache/thrift/pull/193

          Show
          ASF GitHub Bot added a comment - Github user cdwijayarathna closed the pull request at: https://github.com/apache/thrift/pull/193
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1263 (See https://builds.apache.org/job/Thrift/1263/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev e26db2fcf2f7853aaa076f7e299dcdb433029cd1)

          • lib/hs/Thrift.cabal
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1263 (See https://builds.apache.org/job/Thrift/1263/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev e26db2fcf2f7853aaa076f7e299dcdb433029cd1) lib/hs/Thrift.cabal
          Hide
          Chamila Dilshan Wijayarathna added a comment -

          I think it will be better to have some documentation to keep track on what tests are already there and what are missing. I have created https://docs.google.com/spreadsheet/ccc?key=0AolXxLs9J7hydEFMVmRmNl9RY3MwVFNtbnJSM2ZOSUE&usp=drive_web#gid=0 with what I did during my GSoC. We can get help from it if necessary.

          Show
          Chamila Dilshan Wijayarathna added a comment - I think it will be better to have some documentation to keep track on what tests are already there and what are missing. I have created https://docs.google.com/spreadsheet/ccc?key=0AolXxLs9J7hydEFMVmRmNl9RY3MwVFNtbnJSM2ZOSUE&usp=drive_web#gid=0 with what I did during my GSoC. We can get help from it if necessary.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Thrift #1331 (See https://builds.apache.org/job/Thrift/1331/)
          THRIFT-847 Test Framework harmonization across all languages (roger: rev bea273484d63e5c14481754da9ff786b12835471)

          • test/test.sh
            THRIFT-847 Test Framework harmonization across all languages (roger: rev dace6937a7c1f53c45c005f0d5d6e2851b656da6)
          • test/tests.json
            THRIFT-847 Test Framework harmonization across all languages (roger: rev d92179129a5674784da7248f09bd41d32bb762bc)
          • test/test.sh
          • test/hs/TestClient.hs
          • test/hs/TestServer.hs
          • contrib/installDependencies.sh
          • lib/hs/Thrift.cabal
            THRIFT-847 Test Framework harmonization across all languages (roger: rev 7191bc99fe3de1027f7fab21232d5bc6ed1d5db5)
          • test/hs/TestClient.hs
          • test/hs/TestServer.hs
            THRIFT-847 Test Framework harmonization across all languages (roger: rev e26db2fcf2f7853aaa076f7e299dcdb433029cd1)
          • lib/hs/Thrift.cabal
          Show
          Hudson added a comment - SUCCESS: Integrated in Thrift #1331 (See https://builds.apache.org/job/Thrift/1331/ ) THRIFT-847 Test Framework harmonization across all languages (roger: rev bea273484d63e5c14481754da9ff786b12835471) test/test.sh THRIFT-847 Test Framework harmonization across all languages (roger: rev dace6937a7c1f53c45c005f0d5d6e2851b656da6) test/tests.json THRIFT-847 Test Framework harmonization across all languages (roger: rev d92179129a5674784da7248f09bd41d32bb762bc) test/test.sh test/hs/TestClient.hs test/hs/TestServer.hs contrib/installDependencies.sh lib/hs/Thrift.cabal THRIFT-847 Test Framework harmonization across all languages (roger: rev 7191bc99fe3de1027f7fab21232d5bc6ed1d5db5) test/hs/TestClient.hs test/hs/TestServer.hs THRIFT-847 Test Framework harmonization across all languages (roger: rev e26db2fcf2f7853aaa076f7e299dcdb433029cd1) lib/hs/Thrift.cabal

            People

            • Assignee:
              Roger Meier
              Reporter:
              Roger Meier
            • Votes:
              1 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:

                Development