QtTest driver #110

konserw · 2016-03-18T22:39:42Z

It's QtTest based test driver for cucumber-cpp.
I'm not sure if unit test for that is OK.
Looking forward for any comments ;)

konserw · 2016-03-19T10:47:45Z

I've rebased to clean up commit history - now I believe it's ready for merging.
I've switched travis build to qt5 as qttestdriver I've created is qt5-only (pretty sure it is easy to port to older qt also) - should we add both qt4 and 5 to travis build matrix ?

paoloambrosio · 2016-03-27T13:27:40Z

tests/integration/drivers/QtTestDriverTest.cpp

+        QCOMPARE(framework.isCleanedUp, false);
+        QTest::qExec(&framework);
+        QCOMPARE(framework.isInitialized, true);
+        QCOMPARE(framework.isCleanedUp, true);


This is not testing the driver. QtTestSteps has nothing to do with the driver.

I've used boost test as example - how this is different?

class BoostDriverTest : public DriverTest { public: virtual void runAllTests() { stepInvocationInitsBoostTest(); DriverTest::runAllTests(); } private: void stepInvocationInitsBoostTest() { std::cout << "= Init =" << std::endl; using namespace boost::unit_test; BoostStepDouble step; expectFalse("Framework is not initialized before the first test", framework::is_initialized()); step.invokeStepBody(); expectTrue("Framework is initialized after the first test", framework::is_initialized()); } }; int main(int argc, char **argv) { BoostDriverTest test; return test.run(); }

The tests for the Boost driver exercise a test double that inherits from the class under test (instead of exercising the framework directly). Assertions instead check properties of the test framework to make sure that the .invokeStepBody() method did something on the system.

class BoostStepDouble : public BoostStep { VS class QtTestSteps : public QObject {
step.invokeStepBody(); VS QTest::qExec(&framework);

paoloambrosio · 2016-03-27T13:47:41Z

Apart from the comments, nice PR!

paoloambrosio · 2016-03-27T13:50:06Z

...and don't forget to add something in the library dependencies of the README file!

konserw · 2016-04-02T20:30:30Z

I've updated readme, rebased it and partly fixed file writting problem.
Could you give me some advice about how to produce good driver test?
EDIT: Don't mind above - I think I've figured it out ;) Expect commit soon.

konserw · 2016-04-03T07:04:31Z

Any idea on why would this test pass on gcc and fail on clang?
And pass on apple's clang? :D

konserw · 2016-04-03T19:53:58Z

Stupid mistake - those bool variables were uninitialized, that defaults to false (0) on gcc, but not on clang

konserw · 2016-07-18T15:25:05Z

Anyone any comments?

konserw · 2016-08-11T10:37:44Z

If no comments merge please :)

konserw · 2016-09-28T07:27:16Z

Just rebased against current master

konserw · 2016-09-28T08:09:01Z

Rebased and all green - ready to merge ;)

konserw · 2016-09-28T08:16:57Z

One last commit - for compatibility with Qt >= 5.7 c++11 has to be enabled. I wasn't sure if new PR would be better for this...

paoloambrosio · 2016-12-28T15:58:36Z

I have to say I don't like the Posix implementation at all:

don't like temprarily redirecting the entire process's stdout
it has been copied from stack overflow, that is at the very top of my list of things no developer should do

You are right: there is no way out of the horribly closed implementation that Qt has done (shame on them!). On a Posix system we could create a new process running the same code base (fork) and read from its output, but that is not supported by Windows. At this point the lesser of all evils is to use the temporary file solution for all implementations :-(

paoloambrosio

Hopefully last comments before it can be merged :-)

paoloambrosio · 2016-12-28T16:00:59Z

src/drivers/QtTestDriver.cpp

+    if (file.open()) {
+        fileName = file.fileName();
+    }
+    file.close();


If the file cannot be opened, the filename will be null and nothing would work. Perhaps we should stop here and return a failure.

Also can the file be opened in QIODevice::ReadOnly?

paoloambrosio · 2016-12-28T16:02:24Z

src/drivers/QtTestDriver.cpp

+        return InvokeResult::success();
+    else
+    {
+        file.open();


If we close the file at the end (QTemporaryFile's destructor does it) we don't need to re-open it. Also please mind the coding conventions (spaces, braces, ...):

if (...) { ... } else { ... }

paoloambrosio · 2016-12-28T16:07:19Z

src/drivers/QtTestDriver.cpp

+    {
+        file.open();
+        QTextStream ts(&file);
+        return InvokeResult::failure(ts.readAll().toLatin1());


Why toLatin1? I'm no Qt expert but toLocal8Bit looks more like the OS's character set... am I wrong?

paoloambrosio · 2016-12-28T16:09:53Z

include/cucumber-cpp/internal/drivers/QtTestDriver.hpp

+
+class QtTestStep : public BasicStep{
+public:
+    QtTestStep(): IsInitialized(false), IsCleanedUp(false), TestRun(false) {}


Is there a reason why here and in several other places you are using UpperCamelCase instead of lowerCamelCase?

paoloambrosio · 2016-12-28T17:20:38Z

I'm not sure but I believe that there could be an alternative to writing a real file, that is to write the output to a named pipe (available on both Windows and Posix systems, but with different APIs). Happy to merge this with file based I/O as a multi-platform pipe implementation is not straightforward.

konserw · 2017-01-01T22:14:40Z

Please take a look at new implementation using pipes for both POSIX and windows.
It doesn't address both your considerations as this is also mostly copied from SO and it's redirecting entire stdout, but for sure it is better than previous implementation. If you still don't like it I'll prepare temporary-file based version.
I've also rebased against current master and enabled Qt to be found by CMake on windows. Unfortunately travis OSX image does not contain Qt, and building it from source would be quite a burden I think, see: travis-ci/travis-ci#5278
By the way I moved building commands from appveyor.yml to appveyor.bat
EDIT: of course as you can see from commit log I've also added QtTest steps for calc example :)

konserw · 2017-01-02T09:44:23Z

I've also experimented with moving qttest::qexec to another thread but then first step from calcqt example always fail (widget does not render, so label is empty), you can check here: https://github.com/konserw/cucumber-cpp/tree/qttestthread
console apps seem to work, the problem is CalculatorWidget that should be created in main thread (also known in qt as GUI thread... )

marco-piccolino · 2017-01-25T13:58:03Z

@konserw for pre-built Qt on Travis CI you can take a look here: https://github.com/benlau/qtci

Use pipe on POSIX systems to get output from QtTestDriver Use pipe to get output from QtTest also on Windows Pipe abstraction layer moved to separate header Included also QtTest steps for Calc and CalcQt examples

…t file from yml

konserw · 2017-03-30T11:59:12Z

I'm closing this PR as I've splitted it into 2 new:
#140 for CI and Cmake base for just CalcQt example
#142 for driver itself

konserw force-pushed the rebaseqttest branch from 3d26577 to 9359e60 Compare March 19, 2016 10:45

paoloambrosio modified the milestone: v0.3.2 Mar 26, 2016

paoloambrosio reviewed Mar 27, 2016
View reviewed changes

konserw force-pushed the rebaseqttest branch from 80408f0 to eecea9c Compare April 2, 2016 20:28

konserw force-pushed the rebaseqttest branch from beb427b to 9ed70ac Compare September 28, 2016 07:25

paoloambrosio reviewed Dec 28, 2016

View reviewed changes

konserw force-pushed the rebaseqttest branch 2 times, most recently from 601a020 to be94cb7 Compare January 1, 2017 22:04

konserw force-pushed the rebaseqttest branch 3 times, most recently from 462150a to 77e0a3b Compare January 28, 2017 16:25

konserw force-pushed the rebaseqttest branch from 77e0a3b to 1b5abba Compare March 14, 2017 11:42

konserw added 4 commits March 22, 2017 22:24

QtTest based driver for cucumber, including tests

9c23f5b

Use pipe on POSIX systems to get output from QtTestDriver Use pipe to get output from QtTest also on Windows Pipe abstraction layer moved to separate header Included also QtTest steps for Calc and CalcQt examples

Enable Qt usage on appveyor, move appveyor build steps to separate ba…

75ff7b5

…t file from yml

Enable Qt usage on appveyor, move appveyor build steps to separate ba…

e133254

…t file from yml

Enable Qt usage on appveyor, move appveyor build steps to separate ba…

cc2656d

…t file from yml

konserw force-pushed the rebaseqttest branch from 1b5abba to cc2656d Compare March 22, 2017 21:28

get rid of clang warning

3c2cccb

konserw closed this Mar 30, 2017

muggenhor removed this from the v0.4.1 milestone Jun 3, 2017

konserw deleted the rebaseqttest branch April 2, 2018 10:06

QtTest driver #110

QtTest driver #110

Uh oh!

Conversation

konserw commented Mar 18, 2016

Uh oh!

konserw commented Mar 19, 2016

Uh oh!

paoloambrosio Mar 27, 2016

Choose a reason for hiding this comment

Uh oh!

konserw Mar 27, 2016

Choose a reason for hiding this comment

Uh oh!

paoloambrosio Mar 27, 2016

Choose a reason for hiding this comment

Uh oh!

paoloambrosio commented Mar 27, 2016

Uh oh!

paoloambrosio commented Mar 27, 2016

Uh oh!

konserw commented Apr 2, 2016

Uh oh!

konserw commented Apr 3, 2016

Uh oh!

konserw commented Apr 3, 2016

Uh oh!

konserw commented Jul 18, 2016

Uh oh!

konserw commented Aug 11, 2016

Uh oh!

konserw commented Sep 28, 2016

Uh oh!

konserw commented Sep 28, 2016

Uh oh!

konserw commented Sep 28, 2016

Uh oh!

paoloambrosio commented Dec 28, 2016

Uh oh!

paoloambrosio left a comment

Choose a reason for hiding this comment

Uh oh!

paoloambrosio Dec 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paoloambrosio Dec 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paoloambrosio Dec 28, 2016

Choose a reason for hiding this comment

Uh oh!

paoloambrosio Dec 28, 2016

Choose a reason for hiding this comment

Uh oh!

paoloambrosio commented Dec 28, 2016

Uh oh!

konserw commented Jan 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

konserw commented Jan 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marco-piccolino commented Jan 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

konserw commented Mar 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

paoloambrosio Dec 28, 2016 •

edited

Loading

paoloambrosio Dec 28, 2016 •

edited

Loading

konserw commented Jan 1, 2017 •

edited

Loading

konserw commented Jan 2, 2017 •

edited

Loading

marco-piccolino commented Jan 25, 2017 •

edited

Loading