Re: [Geany-Devel] Helping Geany move forward: testing

[Lex Trotman]

As an exercise I scanned the top few (highest numbered) PRs to assess
their commitability from MY personal point of view, found one
immediately committable and did, the rest are:

#1482 still open question if it should revert to previous bad behaviour.

#1481 work in progress

#1478 improvement suggested but commitable then

#1471 havn't had time to look closely, lots of files modified (ok many
are icons, but still) and not a feature that I would test in my
workflow

#1470 havn't looked at it closely, but at first glance its ok, has an
open "cannot reproduce" on a test report, but I don't use snippets, so
it would only get cursory testing by me

#1465 I have only a vague idea what its doing and no idea how to test
it other than compiling it (which Travis has already done)

#1461 and #1457 work in progress

#1456 simply havn't had time to look at it

#1450 suggested wiki instead of adding to core as others have
criticised adding more small filetypes to Geany, undecided

#1445 review tantrum (see comments on it) :)

#1430 has unfixed comments and Travis failures

#1414 support the idea, but its a big change, in a sensitive area
(writing files safely is the PRIMARY purpose of an editor), and I
don't have any networked files to test with.  Also although it
explicitly doesn't change handling on Windows it would need testing to
make sure it didn't accidentally break something there.

#1402 don't know VHDL and testing it would need testing it didn't
affect anything else so time issues and needs actual test material

#1400 still has a review open (though the changes have been made I
think) simply needs time to test there are no unexpected effects of
the signal change

That will do, spent more time than I wanted already.  I guess there
are only a couple that are specifically testing related.  Some more
are due to the problem Matthew pointed out, don't want to break
master, so cautious of complex seeming changes.  The rest are in the
OPs court.

Cheers
Lex
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] Helping Geany move forward: testing

[Matthew Brush]

On 2017-04-28 06:35 PM, Lex Trotman wrote:

...


Geany is almost entirely an interactive application, so until
interactive tests are possible I don't think technical tests like
these will add a great deal to the committability of PRs.



If the tests just test functions, all it needs is to get Geany started up,
then the tests can call the new/changed functions testing with different
inputs and such. There are at least two PRs to do similar.


Sadly Geany isn't a pure functional program, most functions leave
messy side-effects on global data, the Scintilla buffers :(

So you need to be able to examine those.




You can, the tests are just regular extra functions called at runtime, 
you have access to all state that normal code does, it just makes it 
more trouble to setup/assert that state. When you have this in mind 
while writing a test for the new/changed function, you're more likely to 
make it more "pure" and single-task specific. The end result is better 
code and more testable code, which would gradually spread through parts 
of the codebase.





Clangalizers and sanitizers and formatters won't tell you that the PR
actually puts 'z' in the buffer instead of 'a'.



No, but they'll catch a number of runtime bugs that are often hard to
identify upon basic code inspection or manual testing.


Perhaps Columban knows more about using the accessibility framework
for testing now Scintilla supports it?



There are several UI testing frameworks that work with GTK+, though I've not
used any: autopilot, dogtail, and LDTP.

I don't think we really need fully automatic UI testing (seems like too much
work), but we could get a long way just testing at the function level,
ensuring functions uphold their contract and flexing them with unusual
inputs. Making a testable function usually means writing it better too,
avoiding global state and writing more "pure" functions, and making
functions do one thing and not writing huge functions or many small
functions.


We really NEED automatic UI testing and we NEED function unit testing,
but realistically we are not going to get either.  If we don't have
enough resources to just run and test PRs we don't have the resources
to add these.



The contributors add the tests flexing the PR changes, giving the person 
merging the change more confidence and less reason to test every little 
corner case themselves, and are automated and repeatable to ensure the 
assumptions those tests make are not broken by other changes in the 
future. Instead of as the OP suggested, writing up a prose testing 
report by hand, they just write a test function that tests the 
assumptions they have checked, also showing missing assumptions.


Regards,
Matthew Brush
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] Helping Geany move forward: testing

[Lex Trotman]

...
>>
>> Geany is almost entirely an interactive application, so until
>> interactive tests are possible I don't think technical tests like
>> these will add a great deal to the committability of PRs.
>
>
> If the tests just test functions, all it needs is to get Geany started up,
> then the tests can call the new/changed functions testing with different
> inputs and such. There are at least two PRs to do similar.

Sadly Geany isn't a pure functional program, most functions leave
messy side-effects on global data, the Scintilla buffers :(

So you need to be able to examine those.

>
>> Clangalizers and sanitizers and formatters won't tell you that the PR
>> actually puts 'z' in the buffer instead of 'a'.
>>
>
> No, but they'll catch a number of runtime bugs that are often hard to
> identify upon basic code inspection or manual testing.
>
>> Perhaps Columban knows more about using the accessibility framework
>> for testing now Scintilla supports it?
>>
>
> There are several UI testing frameworks that work with GTK+, though I've not
> used any: autopilot, dogtail, and LDTP.
>
> I don't think we really need fully automatic UI testing (seems like too much
> work), but we could get a long way just testing at the function level,
> ensuring functions uphold their contract and flexing them with unusual
> inputs. Making a testable function usually means writing it better too,
> avoiding global state and writing more "pure" functions, and making
> functions do one thing and not writing huge functions or many small
> functions.

We really NEED automatic UI testing and we NEED function unit testing,
but realistically we are not going to get either.  If we don't have
enough resources to just run and test PRs we don't have the resources
to add these.

Hence my suggestions of purely social engineering in previous posts.

Cheers
Lex

>
>
> Regards,
> Matthew Brush
>
> ___
> Devel mailing list
> Devel@lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] Helping Geany move forward: testing

[Matthew Brush]

On 2017-04-28 05:35 PM, Lex Trotman wrote:

On 29 April 2017 at 09:55, Matthew Brush  wrote:

On 2017-04-28 02:35 PM, Thomas Martitz wrote:


Am 27.04.2017 um 22:51 schrieb Vasiliy Faronov:


Hi all,

 From discussions elsewhere, such as [1], it sounds like one of the
things holding back Geany development right now is a need for more
testing.



Helping to test PRs is truly needed, and much appreciated.

However, I do think that Geany lacks also actual developers that cna
merge stuff. I feel the current team is afraid of merging non-trivial
changes, leaving even semi-complex patches to Colomban. Unfortunately
Colomban has little time these days, too, so we're kind of stuck. There
are lots of PRs that have recent activity from the authors and are
tested appropriately but still don't get attention from developers.



My general problem is that we don't have a unstable/development branch per
se, nor proper automated testing, and I don't want to break master so I
won't merge a single thing without testing it thoroughly myself. This can
turn a 5-10 minute merge into a several hours or more testing session,
requiring special setups and re-compiling Geany on 3 different OSes, etc.


I have to agree with Matthew that:

1. Nobody wants to break master because its what everybody is using.
Problem is that if we had a development branch nobody would be using
it because it might break, so its insufficiently tested.  I don't have
a solution to that.

2. I am more willing to accept others testing and to make a judgement
call about testing on all platforms.  I have used that approach
successfully on other projects where I couldn't personally test some
configurations.  But I understand where Matthew is coming from
regarding the amount of work to do a good testing job.

3. A thorough test is becoming too big a job, and that is even worse
for the more complex PRs that Thomas mentions.  Simply don't have the
time.  And for changes to the plugin interface that need a plugin to
test, well, unless the OP provides such a plugin, it just isn't going
to happen.



Travis CI is great, but unless it can run make check with loads of static
analysis and runtime analysis while it runs unit tests and such, it's
basically just saying the code compiles. As we all know, it's relatively
easy to make C code that compiles but crashes horribly at runtime in weird
corner cases (off by one, null deref, etc.).

Personally I'd feel a lot better merging PRs I haven't thoroughly tested if
we had:

  - Clang static analyzer during the build
  - A Git hook or manual use of clang-format or other formatter to prevent
the "extra white space" or "wrong comment style" type of issues that
commonly occur in PRs.
  - Ability for PRs to come with tests (requires testing support).
  - Linking in Clang's address & memory sanitizers while running all of the
tests.


Geany is almost entirely an interactive application, so until
interactive tests are possible I don't think technical tests like
these will add a great deal to the committability of PRs.


If the tests just test functions, all it needs is to get Geany started 
up, then the tests can call the new/changed functions testing with 
different inputs and such. There are at least two PRs to do similar.



Clangalizers and sanitizers and formatters won't tell you that the PR
actually puts 'z' in the buffer instead of 'a'.



No, but they'll catch a number of runtime bugs that are often hard to 
identify upon basic code inspection or manual testing.



Perhaps Columban knows more about using the accessibility framework
for testing now Scintilla supports it?



There are several UI testing frameworks that work with GTK+, though I've 
not used any: autopilot, dogtail, and LDTP.


I don't think we really need fully automatic UI testing (seems like too 
much work), but we could get a long way just testing at the function 
level, ensuring functions uphold their contract and flexing them with 
unusual inputs. Making a testable function usually means writing it 
better too, avoiding global state and writing more "pure" functions, and 
making functions do one thing and not writing huge functions or many 
small functions.


Regards,
Matthew Brush

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] Helping Geany move forward: testing

[Lex Trotman]

On 29 April 2017 at 09:55, Matthew Brush  wrote:
> On 2017-04-28 02:35 PM, Thomas Martitz wrote:
>>
>> Am 27.04.2017 um 22:51 schrieb Vasiliy Faronov:
>>>
>>> Hi all,
>>>
>>>  From discussions elsewhere, such as [1], it sounds like one of the
>>> things holding back Geany development right now is a need for more
>>> testing.
>>
>>
>> Helping to test PRs is truly needed, and much appreciated.
>>
>> However, I do think that Geany lacks also actual developers that cna
>> merge stuff. I feel the current team is afraid of merging non-trivial
>> changes, leaving even semi-complex patches to Colomban. Unfortunately
>> Colomban has little time these days, too, so we're kind of stuck. There
>> are lots of PRs that have recent activity from the authors and are
>> tested appropriately but still don't get attention from developers.
>>
>
> My general problem is that we don't have a unstable/development branch per
> se, nor proper automated testing, and I don't want to break master so I
> won't merge a single thing without testing it thoroughly myself. This can
> turn a 5-10 minute merge into a several hours or more testing session,
> requiring special setups and re-compiling Geany on 3 different OSes, etc.

I have to agree with Matthew that:

1. Nobody wants to break master because its what everybody is using.
Problem is that if we had a development branch nobody would be using
it because it might break, so its insufficiently tested.  I don't have
a solution to that.

2. I am more willing to accept others testing and to make a judgement
call about testing on all platforms.  I have used that approach
successfully on other projects where I couldn't personally test some
configurations.  But I understand where Matthew is coming from
regarding the amount of work to do a good testing job.

3. A thorough test is becoming too big a job, and that is even worse
for the more complex PRs that Thomas mentions.  Simply don't have the
time.  And for changes to the plugin interface that need a plugin to
test, well, unless the OP provides such a plugin, it just isn't going
to happen.


> Travis CI is great, but unless it can run make check with loads of static
> analysis and runtime analysis while it runs unit tests and such, it's
> basically just saying the code compiles. As we all know, it's relatively
> easy to make C code that compiles but crashes horribly at runtime in weird
> corner cases (off by one, null deref, etc.).
>
> Personally I'd feel a lot better merging PRs I haven't thoroughly tested if
> we had:
>
>   - Clang static analyzer during the build
>   - A Git hook or manual use of clang-format or other formatter to prevent
> the "extra white space" or "wrong comment style" type of issues that
> commonly occur in PRs.
>   - Ability for PRs to come with tests (requires testing support).
>   - Linking in Clang's address & memory sanitizers while running all of the
> tests.

Geany is almost entirely an interactive application, so until
interactive tests are possible I don't think technical tests like
these will add a great deal to the committability of PRs.
Clangalizers and sanitizers and formatters won't tell you that the PR
actually puts 'z' in the buffer instead of 'a'.

Perhaps Columban knows more about using the accessibility framework
for testing now Scintilla supports it?

Cheers
Lex

>
> Just some thoughts.
>
> Regards,
> Matthew Brush
>
> ___
> Devel mailing list
> Devel@lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] Helping Geany move forward: testing

[Matthew Brush]

On 2017-04-28 02:35 PM, Thomas Martitz wrote:

Am 27.04.2017 um 22:51 schrieb Vasiliy Faronov:

Hi all,

 From discussions elsewhere, such as [1], it sounds like one of the
things holding back Geany development right now is a need for more
testing.


Helping to test PRs is truly needed, and much appreciated.

However, I do think that Geany lacks also actual developers that cna
merge stuff. I feel the current team is afraid of merging non-trivial
changes, leaving even semi-complex patches to Colomban. Unfortunately
Colomban has little time these days, too, so we're kind of stuck. There
are lots of PRs that have recent activity from the authors and are
tested appropriately but still don't get attention from developers.



My general problem is that we don't have a unstable/development branch 
per se, nor proper automated testing, and I don't want to break master 
so I won't merge a single thing without testing it thoroughly myself. 
This can turn a 5-10 minute merge into a several hours or more testing 
session, requiring special setups and re-compiling Geany on 3 different 
OSes, etc.


Travis CI is great, but unless it can run make check with loads of 
static analysis and runtime analysis while it runs unit tests and such, 
it's basically just saying the code compiles. As we all know, it's 
relatively easy to make C code that compiles but crashes horribly at 
runtime in weird corner cases (off by one, null deref, etc.).


Personally I'd feel a lot better merging PRs I haven't thoroughly tested 
if we had:


  - Clang static analyzer during the build
  - A Git hook or manual use of clang-format or other formatter to 
prevent the "extra white space" or "wrong comment style" type of issues 
that commonly occur in PRs.

  - Ability for PRs to come with tests (requires testing support).
  - Linking in Clang's address & memory sanitizers while running all of 
the tests.


Just some thoughts.

Regards,
Matthew Brush
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Re: [Geany-Devel] Helping Geany move forward: testing

[Thomas Martitz]

Am 27.04.2017 um 22:51 schrieb Vasiliy Faronov:

Hi all,

 From discussions elsewhere, such as [1], it sounds like one of the
things holding back Geany development right now is a need for more
testing.


Helping to test PRs is truly needed, and much appreciated.

However, I do think that Geany lacks also actual developers that cna 
merge stuff. I feel the current team is afraid of merging non-trivial 
changes, leaving even semi-complex patches to Colomban. Unfortunately 
Colomban has little time these days, too, so we're kind of stuck. There 
are lots of PRs that have recent activity from the authors and are 
tested appropriately but still don't get attention from developers.


So I think we need more people that can push code to Geany directly, 
effectively dividing the workload onto more people. It's just too much 
work for a single developer, especially these days.


Unless this situation improves, I'm afraid that intensive testing of PRs 
is nice but kind of a wasted effort. This is worsened by the fact that 
"unpreviliged" testers can't assign labels in Github, it's really hard 
to get an overview about which PRs have received extended testing.


In the meantime, we're scaring contributors away because contributes 
aren't looked at in a timely manner.


Take this as an application. I would love to actively help if I'm 
granted push or github-label-set access.


Best regards.
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel