Re: [QGIT RFC] Unit tests for QGit

Previous thread: [TopGit PATCH 2/2] tg-import.sh: A dump quilt queue importer by Bert Wesarg on Friday, August 8, 2008 - 11:19 am. (4 messages)

Next thread: [PATCH] ReleaseNotes: git-gui is not installed in $PATH by Pieter de Bie on Friday, August 8, 2008 - 3:22 pm. (1 message)
From: Jan Hudec
Date: Friday, August 8, 2008 - 2:13 pm

Hello Marco and others,

I've been thinking about some refactoring of QGit since some time. And to be
sure I don't screw up things too hard in the process, I thought about adding
a test suite infrastructure first (and add some test cases for each think
just before refactoring it).

The problem is, that implementing unittests means I need to compile
2 separate binaries -- qgit itself and the test -- using most (but not all)
of the same sources. I see two ways to do it, so I'd like to ask which you
consider cleaner:

 1. Reorganize stuff so that a (static) library is created from all the
    sources except qgit.cpp and than qgit.cpp is linked to this library to
    create qgit and the tests are linked with it to provide the test runner.

    Pros:
     - The .pro files should remain reasonably simple.
     - The sources are only compiled once.
    Cons:
     - Need to split the src directory to two, so bigger moving stuff around.

 2. Put the list of sources into file included in the src.pro and include it
    in the tests.pro file too.

    Pros:
     - No libraries and stuff
     - Less moving stuff around.
    Cons:
     - The sources actually get compiled twice, once for the tests and once
       for the qgit binary.
     - Paths to the sources need to be manually adjusted after including into
       the .pro files, making the .pro files rather ugly.

There seems to be no solution requiring less changes to the projects, because
qmake can only create one library or executable per directory and including
files from other directory is not supported to well.

I've already done the later (have patch series ready), but I am now thinking
that I should probably redo it the first way. What do you think. Does it make
sense to do that?

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--

From: Benjamin Sergeant
Date: Friday, August 8, 2008 - 4:00 pm

Maybe you can have a look at QTestLib. But it won't solve your
buildsystem issues. You'll need one .pro per test. (I have one .pro
per test plus one directory per test). There's probably other ways to
using it.

http://doc.trolltech.com/4.4/qtestlib-manual.html#qtestlib
--

From: Jan Hudec
Date: Sunday, August 10, 2008 - 12:55 am

Sure I did. Unfortunately they don't suggest any good way to handle your
build process with it in their examples. Seems to me they never tried testing
an application with it.

I plan to go down the QTestLib route. Maybe it could be combined with
LDTP[1] for blackbox testing -- they claim to be able to use Qt 4's

Depends on what you call a test. But generally there should be no reason to
have more than one .pro file for all tests. You just need to manually
maintain a list of test classes or create some kind of static instance

[1] http://ldtp.freedesktop.org/

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--

From: Marco Costalba
Date: Sunday, August 17, 2008 - 1:57 am

Hi Jan,

  sorry to reply so late but I just returned from holiday (no PC there

That's interesting. I have NO experience on test suites for GUI
applications (command line applications like git I would think are

This is not a cons IMHO if it helps in separating tests from sources.

As I said I am no expert, but I would try to

- Let the test suite be easily stripped/not compiled for the
publishing (remember that we have to produce also that little
qgit_install.exe file used on *that* OS)

- Let the test be compiled only on demand (during developing I just
want to compile and run as few things as possible: C++ is already
quite bad in that regard and I don't want the situation get worst. BTW
I consider C++ slow compile times the biggest and probably only
drawback of C++ against C for big projects)

- Try to find some literature/reference before starting coding. As I
said I am no expert of GUI testing, so I would probably try to find
some Qt projects that use it and see/ask the developers how they
managed to do that and what are the problems. Then try to be stick to
known best practice (read someone that has DONE that in a REAL
project, not someone that has WRITTEN about that in a paper or a
vendor marketing/documentation)

Anyhow I'm really interested in this thing, and hope to see your work
soon. Please feel free to drop me a line for any help you think I can
give you.

Bye
Marco
--

From: Jan Hudec
Date: Sunday, August 17, 2008 - 7:15 am

Well, there are basically three points at which GUI application can be
tested:
 1. The code that provides data does not directly do anything with graphics
    and therefore can be tested by normal unittests. In this case such tests
    can be written for the Git and related classes.
 2. Widget events can be emulated inside the test driver, for which Qt4
    contains a (quite basic but usable) QTestlib module. The tests work as
    normal unittests.
 3. User interaction can be simulated from outside the application, eg. with
    LDTP.

I actually plan the first two items. While the third would be less invasive
(no need to link anything anywhere), unittests are much more useful for

The tests would be a separate directory in any case. What I will need to
separate is qgit.cpp from all other sources. So there will be *three* folders
in the end -- one for linking the qgit binary, containing only qgit.cpp and
a .pro file, one for the majority of source and one for linking the testsuite

The test suite basically must be a separate binary. At least that's the only
method I ever used -- it would be possible to link everything together and

Yes, you can just comment out the tests. On the other hand it's during the
development I normally want to run the tests. I usually prefer to write
a test for things I work on over starting the user interface and testing them
manually, because manual testing is much more prone to forgetting some

Well, KDE people talked about doing it, but I am not sure how much they

Bye,
Jan

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--

From: Marco Costalba
Date: Sunday, August 17, 2008 - 8:46 am

Could you please post somewhere the patches ?

Better yet to fork from http://repo.or.cz/w/qgit4.git and set up your
tree on http://repo.or.cz/ host (it's easy and fast, thanks Peter :-)

I can check that and eventually pulling from that.

As a general rule if you have already done a good chunk of work with
unit test patches I would avoid to ask you to redo in a different way,
so I would say it does not make a lot of sense to me at least before
looking at the code.


Marco

P.S: I have played a bit with qmake some time ago (to set-up the
double build environment Windows/Linux) so perhaps I could help you in
finding some useful trick to avoid the cons regarding .pro files you
posted. But of course I first need to see the patches.
--

From: Jan Hudec
Date: Sunday, August 17, 2008 - 12:58 pm

I only have the basic infrastructure (building and basic runner) ready, while
I am trying to figure out how individual parts of QGit are supposed to work.
But I can publish that, yes.

We had a very busy time at work lately, so I didn't get to it much. I'll try

Makes sense. git://repo.or.cz/qgit4/bulb.git

But as I said, I only have basic infrastructure and am currently looking at
what to write tests for and how exactly that test should work. The detection
of git vs. stgit branch (does not work for me) is likely first candidate, but
that is not UI thing. Maybe the effects of that option on the UI are the next
(I would like to make them more localized somehow, though I don't know how
yet). Other likely candidate would be anything which could be affected by
funny filenames (containing spaces, newlines, quotes, backslashes, control

I was testing what can be done so far. So now I decided to go the library way
(in scons or manually written makefiles I would just re-link the same .o

Well, I somehow managed -- except I am not sure I dealed with the windows
part correctly. What could be improved is maybe if you know how to signal
a dependency between two projects. I currently rely on the top-level makefile
always calling the subdirs in the order they are specified, but I fear
portable recursive make does not really offer any better solution, so qmake
can't really do that either.

Unfortunately while Qt is generally documented quite well, qmake
documentation is not so good.

Note: I think I found a bug in qmake here -- when you run qmake at top level,
the makefile will call qmake in subdirectories to create makefiles there, but
the rule has no dependencies, so it will not remake the makefiles when the
.pro files change there.

Also I don't understand why you set 'MAKEFILE = qmake' in the src/src.pro --
it does not seem to be respected, at least when I call it through the
top-level qgit.pro (which I now have to when there are 3 subdirs).

Regards,
Jan

-- 
						 Jan ...
From: Marco Costalba
Date: Sunday, August 17, 2008 - 1:30 pm

This sounds as a bug. Could you elaborate on that please ?


BTW the test for a StGit repo is:

isStGIT = run("stg branch", &stgCurBranch); // slow command


Could the following help ?


I knew that. For my use I always delete Makefiles after modifying any
of *.pro files, I'm sure it exists a better way but honestly I didn't

From http://doc.trolltech.com/4.0/qmake-variable-reference.html#makefile

MAKEFILE
This variable specifies the name of the Makefile which qmake should
use when outputting the dependency information for building a project.
The value of this variable is typically handled by qmake or qmake.conf
and rarely needs to be modified.

I annotated the src.pro file and I found that line belongs from the
very first version of src.pro, possibly copied from the Qt examples,
so it smells you are right and we could remove that.

Marco
--

From: Jan Hudec
Date: Monday, August 18, 2008 - 11:00 am

Yes, I've seen that command. But it returns true for me even when it's not

Does help a bit. Thanks.
That is, confirms my suspicion that there is no really correct solution and
remembered me the ordered config option, that I noticed in the documentation
once, but wasn't able to find again when I actually wrote the .pro files.


I don't think there's too much to investigate -- it looks like an obvious bug
;-). Unless it's caused by the MAKEFILE setting :-( (I'll have to remove it

Looks like that. Funny thing is, that normally the makefiles are called
Makefile for me, not qmake, but I did see the qmake files (IIRC when I ran
qmake -recursive). That would mean it takes the value from the top-level .pro
and ignores it in the subdirs.

Regards,
Jan

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--

From: Marco Costalba
Date: Tuesday, August 19, 2008 - 7:53 am

That's interesting !

The command just runs "stg branch", in my StGit setup this returns an
error (some stuff written to stderr) if the directory where it is run
is not a StGit stack.

run() just detects the error and returns false.

Could you try to run it from a console and see if you have stuff on stderr ?

Thanks
Marco
--

From: Jan Hudec
Date: Wednesday, August 27, 2008 - 1:18 pm

I am slowly progressing towards writing a test case for it ;-).

Actually, I just wrote a first simple test for it. I didn't find this (now
the stg branch finds out properly), but I found another problem -- when
switching from non-stgit branch to a stgit one, Git::init will not notice,
because the path didn't change, so the check is not re-run. Applies to the

I don't recall the details (it was some time ago) and the repository might
have been a bit screwed up. The point there was the branch /used to be/
a stgit one. So it was rather a problem of stgit keeping duplicate

By the way, I looked at the makefiles again and found that they are actually
regenerated correctly when you change the .pro files. While the master
makefile does not have dependencies for the subdir makefiles, each makefile
does have dependencies for itself. And make always tries to rebuild the
makefile before doing anything else. Therefore it does not work to:
    make src//Makefile
but it *does* work to:
    make -C src Makefile
(*and* it will rebuild the makefile when you 'make debug' or 'make release').

Regards,
Jan

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--

From: Marco Costalba
Date: Thursday, August 28, 2008 - 4:29 am

I have never tested on repos where some branches are under stgit and
others are not. Actually I even didn't know it was possible.

The command:

isStGIT = run("stg branch", &stgCurBranch); // slow command

is used to check if a repo is under StGit control, i.e  'stg init' has
been run in the repo working directory (it doesn't mean that there are
StGit patches applied or unapplied, could be also without them).

So It's not very clear to me what does it mean "switching from
non-stgit branch to a stgit one"

Thanks
Marco
--

From: Karl
Date: Thursday, August 28, 2008 - 8:31 am

StGit has no per-repo data. It's all per-branch. "stg init" operates

Hmm. For me, "stg branch" succeeds even if "stg init" has not yet been
run (which is arguably as it should be, since it doesn't require that
stg init has been run in the current branch). "stg series" or
something is probably better for this purpose.

Though if you're concerned about speed (as the comment indicates), you
should probably do something cheaper than running stg, such as

Switching from a branch where "stg init" hasn't been run, to one where
it has.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
--

From: Marco Costalba
Date: Thursday, August 28, 2008 - 11:54 am

Ok. Thanks. In this case the check qgit does is broken, and I think
not only that because I never had this point clear while developing

But if I run 'stg branch' in a git-only repo this gives an error. This

Actually the actual code chunk is:

        // check for a StGIT stack
         QDir d = gitDir;

         if (d.exists("patches")) { // early skip

                 isStGIT = run("stg branch", &stgCurBranch); // slow command

                 stgCurBranch = stgCurBranch.trimmed();
         } else
                 isStGIT = false;



Indeed I need the Stgit current branch name to filter out the refs
found with a following "git show-ref -d" command:

The code chunk is actually

// run the command and save output in runOutpt
if (!run("git show-ref -d", &runOutput))
       return false;

QStringList refsList = runOutput.split('\n', QString::SkipEmptyParts);

FOREACH_SL (it, refsList) {

      QString revSha = (*it).left(40);
      QString refName = (*it).mid(41);

      // save StGIT patch sha, to be used later
      if (refName.startsWith("refs/patches/" + stgCurBranch + "/")) {

              .... we have found a reference to a StGit patch of
current branch ...
      }

......


Thanks again. I don't know why but I was somehow sticked to the idea
that 'stg init' was behaving similar to 'git init', i.e. you need to
run it only once per repo.

Marco
From: Karl
Date: Thursday, August 28, 2008 - 3:18 pm

Ah. I guess it might have gotten fixed recently, then. [ ... makes
some quick tests ... ] Yes, it gives an error in 0.13, but not in
0.14.

Not failing in this case is arguably correct for stg branch. But stg
series can per definition not work before stg init, so I recommend you
use that instead. Or don't use an stg command at all.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
--

From: Jan Hudec
Date: Thursday, August 28, 2008 - 3:01 pm

That would indeed mean, that the check does not do what indented and would
show the same symptoms I recalled initially. Seems I can finally reproduce

Ook. Ook.

Now I actually wrote the test cases I am begining to understand why it
behaves the way it does. 

So, now there is a test infrastructure plus test case to reproduce this
switching between stgit and non-stgit branch in
git://repo.or.cz/qgit4/bulb.git (http://repo.or.cz/r/qgit4/bulb.git) with
whoping 9 commits. Should I send out a patch series, or do you prefer just
pulling?

Now in my opinion the code could use some refactoring rather than just fixing
the bugs (my long term intent is to add features like topgit support,
push/pull/merge and other things git-gui can do and such). I'd start with
the Git initialization sequence. I'll write tests for the new code, but as
I expect it to have significantly different interface from the old one, I'll
not try to write tests for the current one.

Best regards,
Jan

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
--

From: Marco Costalba
Date: Friday, August 29, 2008 - 12:01 am

We have following possibilities:

- Pull the code directly in qgit master as soon as there are some new
commits in your branch

- Pull the code in public qgit repo but under a different branch,
let's call' it "next" ;-)

- Waiting for your code has stabilized a bit (testing infrastructure
is very young and for what I have seen from revision history code is
still very 'fluid'), then pull the branch in qgit master directly.


These are my two cents:

Option one could be a little bit misleading for people pulling from
qgit repo to get current qgit sources + just fixes.

Option two is doable but is an additional step with an additional
maintainer burden, probably the current number of contributors to qgit
is not enough to justify such a complex development model.

Perhaps option three it seems the more balanced, also looking at
projects git related and with similar size of qgit, as example StGit
itself. When let's say Karl has ready a block of patches he sends them
all as a series and are applied to StGit master branch.

The only modification I would suggest is that I can pull from you repo
directly instead of asking you to send patches to the git list.

I leave up to you when to ask for a pull request, I only ask you to
consider that qgit public repo is pulled also by people not interested
in the latest development, and they only want a stable qgit, so please
ask for a pull when you think stuff is stable enough.

Comments?

Marco
--

Previous thread: [TopGit PATCH 2/2] tg-import.sh: A dump quilt queue importer by Bert Wesarg on Friday, August 8, 2008 - 11:19 am. (4 messages)

Next thread: [PATCH] ReleaseNotes: git-gui is not installed in $PATH by Pieter de Bie on Friday, August 8, 2008 - 3:22 pm. (1 message)