[QGIT RFC] Unit tests for QGit

[QGIT RFC] Unit tests for QGit

[Jan Hudec]

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>

Re: [QGIT RFC] Unit tests for QGit

[Benjamin Sergeant]

On Fri, Aug 8, 2008 at 2:13 PM, Jan Hudec <bulb@ucw.cz> wrote:
> 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>
> --

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

Re: [QGIT RFC] Unit tests for QGit

[Jan Hudec]

On Fri, Aug 08, 2008 at 16:00:57 -0700, Benjamin Sergeant wrote:
> On Fri, Aug 8, 2008 at 2:13 PM, Jan Hudec <bulb@ucw.cz> wrote:
> > 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:
> > [...]
> 
> Maybe you can have a look at QTestLib. But it won't solve your

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
accessibility to control an application.

> 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.

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
self-registration (which I did).

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

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

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

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Fri, Aug 8, 2008 at 10:13 PM, Jan Hudec <bulb@ucw.cz> wrote:
> Hello Marco and others,
>

Hi Jan,

  sorry to reply so late but I just returned from holiday (no PC there
due to it was severely forbidden by my boss aka wife :-)


> 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).
>

That's interesting. I have NO experience on test suites for GUI
applications (command line applications like git I would think are
easier to setup some tests suite for)


> 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.
>

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

Re: [QGIT RFC] Unit tests for QGit

[Jan Hudec]

On Sun, Aug 17, 2008 at 09:57:36 +0100, Marco Costalba wrote:
> On Fri, Aug 8, 2008 at 10:13 PM, Jan Hudec <bulb@ucw.cz> wrote:
>   sorry to reply so late but I just returned from holiday (no PC there
> due to it was severely forbidden by my boss aka wife :-)

That's all right. I am not progressing that fast.

> > 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).
> That's interesting. I have NO experience on test suites for GUI
> applications (command line applications like git I would think are
> easier to setup some tests suite for)

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
debugging, because you can test individual functions independently.

> > 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.
> >
> 
> This is not a cons IMHO if it helps in separating tests from sources.

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
with the test 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)

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
start the test suite using a parameter, but I don't intend to do that.

> - 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)

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
important corner cases.

> - 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)

Well, KDE people talked about doing it, but I am not sure how much they
actually do. But it's really just normal unit-testing.

> 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,
Jan

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

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Fri, Aug 8, 2008 at 10:13 PM, Jan Hudec <bulb@ucw.cz> wrote:

>
> 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?
>

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.

Re: [QGIT RFC] Unit tests for QGit

[Jan Hudec]

On Sun, Aug 17, 2008 at 16:46:34 +0100, Marco Costalba wrote:
> On Fri, Aug 8, 2008 at 10:13 PM, Jan Hudec <bulb@ucw.cz> wrote:
> > 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?
> 
> Could you please post somewhere the patches ?

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
to write some tests soon.

> 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.

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
chars and such).

> 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.

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
files, but qmake does not seem to allow that) to avoid double compilation.

> 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.

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 'Bulb' Hudec <bulb@ucw.cz>

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Sun, Aug 17, 2008 at 8:58 PM, Jan Hudec <bulb@ucw.cz> wrote:
>
> 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)

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

in function Git::getRefs() , file git_startup.cpp


>
> 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.
>

Could the following help ?

http://lists.trolltech.com/qt4-preview-feedback/2004-10/thread00174-0.html

>
> 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.
>

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
investigate too much on this.

> 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).
>

>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

Re: [QGIT RFC] Unit tests for QGit

[Jan Hudec]

On Sun, Aug 17, 2008 at 21:30:46 +0100, Marco Costalba wrote:
> On Sun, Aug 17, 2008 at 8:58 PM, Jan Hudec <bulb@ucw.cz> wrote:
> >
> > 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)
> 
> 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
> 
> in function Git::getRefs() , file git_startup.cpp

Yes, I've seen that command. But it returns true for me even when it's not
a stg branch :-(. I am not sure what the problem there is.

> > 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.
> >
> 
> Could the following help ?
> 
> http://lists.trolltech.com/qt4-preview-feedback/2004-10/thread00174-0.html

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.

Added the option and rewound the branch on repo.or.cz.

> > 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.
> >
> 
> 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
> investigate too much on this.

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
and check)

> > 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).
> >
> 
> >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.

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>

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Mon, Aug 18, 2008 at 7:00 PM, Jan Hudec <bulb@ucw.cz> wrote:
> On Sun, Aug 17, 2008 at 21:30:46 +0100, Marco Costalba wrote:
>> On Sun, Aug 17, 2008 at 8:58 PM, Jan Hudec <bulb@ucw.cz> wrote:
>> >
>> > 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)
>>
>> 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
>>
>> in function Git::getRefs() , file git_startup.cpp
>
> Yes, I've seen that command. But it returns true for me even when it's not
> a stg branch :-(. I am not sure what the problem there is.
>

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

Re: [QGIT RFC] Unit tests for QGit

[Jan Hudec]

On Tue, Aug 19, 2008 at 15:53:05 +0100, Marco Costalba wrote:
> On Mon, Aug 18, 2008 at 7:00 PM, Jan Hudec <bulb@ucw.cz> wrote:
> > On Sun, Aug 17, 2008 at 21:30:46 +0100, Marco Costalba wrote:
> >> On Sun, Aug 17, 2008 at 8:58 PM, Jan Hudec <bulb@ucw.cz> wrote:
> >> >
> >> > 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)
> >>
> >> This sounds as a bug. Could you elaborate on that please ?

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
other direction too, of course.

> >> BTW the test for a StGit repo is:
> >>
> >> isStGIT = run("stg branch", &stgCurBranch); // slow command
> >>
> >> in function Git::getRefs() , file git_startup.cpp
> >
> > Yes, I've seen that command. But it returns true for me even when it's not
> > a stg branch :-(. I am not sure what the problem there is.
> >
> 
> 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.

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
information all over the place.

> run() just detects the error and returns false.

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>

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Wed, Aug 27, 2008 at 10:18 PM, Jan Hudec <bulb@ucw.cz> wrote:
>
> 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
> other direction too, of course.
>

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

Re: [QGIT RFC] Unit tests for QGit

[Karl Hasselström]

On 2008-08-28 13:29:25 +0200, Marco Costalba wrote:

> On Wed, Aug 27, 2008 at 10:18 PM, Jan Hudec <bulb@ucw.cz> wrote:
>
> > 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 other direction too, of
> > course.
>
> 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.

StGit has no per-repo data. It's all per-branch. "stg init" operates
on the current branch, not the whole repo.

> 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).

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
checking if .git/patches/<branchname> exists.

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

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

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Thu, Aug 28, 2008 at 5:31 PM, Karl Hasselström <kha@treskal.com> wrote:
>
> StGit has no per-repo data. It's all per-branch. "stg init" operates
> on the current branch, not the whole repo.
>

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
the interface.

>
> 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.
>

But if I run 'stg branch' in a git-only repo this gives an error. This
conditions, at least until now, has always been working for me.


> Though if you're concerned about speed (as the comment indicates), you
> should probably do something cheaper than running stg, such as
> checking if .git/patches/<branchname> exists.
>

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 ...
      }

......

}



>> So it's not very clear to me what does it mean "switching from
>> non-stgit branch to a stgit one"
>
> Switching from a branch where "stg init" hasn't been run, to one where
> it has.
>

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

Re: [QGIT RFC] Unit tests for QGit

[Jan Hudec]

On Thu, Aug 28, 2008 at 20:54:44 +0200, Marco Costalba wrote:
> On Thu, Aug 28, 2008 at 5:31 PM, Karl Hasselström <kha@treskal.com> wrote:
> > StGit has no per-repo data. It's all per-branch. "stg init" operates
> > on the current branch, not the whole repo.
> 
> 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
> the interface.
> 
> > 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.

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
them.

> But if I run 'stg branch' in a git-only repo this gives an error. This
> conditions, at least until now, has always been working for me.
> 
> 
> > Though if you're concerned about speed (as the comment indicates), you
> > should probably do something cheaper than running stg, such as
> > checking if .git/patches/<branchname> exists.
> >
> 
> 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;

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>

Re: [QGIT RFC] Unit tests for QGit

[Marco Costalba]

On Fri, Aug 29, 2008 at 12:01 AM, Jan Hudec <bulb@ucw.cz> wrote:
>
> 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?
>

I prefer to pull.

> 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.
>

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

Re: [QGIT RFC] Unit tests for QGit

[Karl Hasselström]

On 2008-08-28 20:54:44 +0200, Marco Costalba wrote:

> On Thu, Aug 28, 2008 at 5:31 PM, Karl Hasselström <kha@treskal.com>
> wrote:
>
> > StGit has no per-repo data. It's all per-branch. "stg init"
> > operates on the current branch, not the whole repo.
>
> 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
> the interface.
>
> > 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.
>
> But if I run 'stg branch' in a git-only repo this gives an error.
> This conditions, at least until now, has always been working for me.

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