Call to action: Clean up unit test tree.

[Louis Landry]

Hi guys and gals,

Our code style errors are down to 0 in the library tree (including legacy).  We have a few warnings, but pretty much all of those are underscore prefixed object properties which are breaking changes in some cases, so we are taking our time.  The unit test tree however has over 10,000 code style errors right now!!

I'd like to get a group of people together that are interested in systemically running through these unit test packages and getting them consistent and compliant... any takers?

- Louis

[Donald Gilbert]

I'm up for it. How do you want to organize it, so we aren't duplicating work? Assign sets of tests to clean up to those who volunteer? Or should the volunteers just claim what they want to fix, on a first come first serve basis?

[Chad Windnagle]

Perhaps the proper thing to do would be to create a google spreadsheet with a list of all the libraries, and have people put their names on libraries they are willing to tackle. Then we can mark completed, in progress, etc...

I'd be willing to help out with this as well.

-Chad

Regards,
Chad Windnagle
Fight SOPA

[Donald Gilbert]

I completed unit tests for JModelLegacy - https://github.com/joomla/joomla-platform/pull/1588

[Donald Gilbert]

I am going to start a public google doc to cover this, if no one else has already.

[Donald Gilbert]

And here it is - https://docs.google.com/spreadsheet/ccc?key=0AqKlGgpGs6oOdDlYa21qN3R6ZllLNV83ZllnSUJZZ0E

The first column is the class that requires testing.

The second column is a select list of whether the test is completed. You can select yes, no or partial (if some of the tests are complete)

The third column is the name of the person that would like to be responsible for completing a test.

I'm busy going through all the current unit tests, marking their completeness level in the google doc. So when you go there, anything marked as red is an immediate need, the yellow is partially completed, and red is an immediate need.

[Donald Gilbert]

So I just realized that this was referring to coding standard fixes, and not the tests themselves. :(

Everyone: The google doc I linked to above covers what tests have not been completed within the legacy suite. Those also need done, but we still need to create a google doc for cleaning up the coding standards.

[Louis Landry]

Thanks Donald.

I did mean code style, though I'm certainly not going to complain about getting more tests written, that would be fantastic!

If anyone would like to lead the charge on this and run with it please let me know.  It'd be great to start enforcing coding standards on the unit test tree as well. :-)

- Louis

[Elin Waring]

Whoa someone has been making the world less ugly http://developer.joomla.org/coverage/

So in terms of unit test coverage itself that's where I look for things to do. There is some low hanging fruit such as a lot of expectedException tests and also methods where one "if" never is true in the current tests (and admittedly it is really hard sometimes to come up with the right data for some of those).  There are also tons of places in Jhtml   that are not hard to write.

@Louis is there a reason we can't make a separate project in Jenkins just to run the codestyle checks for the test tree? I'm just saying to do it temporarily until we get to the point that they can just run with the code?

Elin

[Donald Gilbert]

Whoa is right - that code coverage system is very nice!

I'll start using that instead of my paltry google doc list. Do you know if it's live coverage from the staging or master branch of the platform repo?

[Ian]

http://developer.joomla.org/coverage-legacy/ has a coverage report for the legacy tree.  It isn't live - it is a nightly job that runs from the staging branch of the platform repo.

Ian

[Elin Waring]

So I had an idea  ... I made  an issue

https://github.com/joomla/joomla-platform/issues/1597

That I see as a standing issue, and I hope it will make resources and the need for help on this more visible. And  we can avoid duplicate work if people will leave comments naming what they are working on.

We are going to get a good bump in percent test coverage with the merging of the GSoC projects but really if you care about backward compatibility you need to care about getting test coverage of older classes. 


Elin

[Donald Gilbert]

Thanks for taking the initiative on this Elin - I've added what I'm working on.

[Elin Waring]

In terms of cleaning up the tree .. we really need some sniffs specifically for that tree if it is going to work. For example, the biggest issue I see is line length. Tests have a lot of issues with line length because the expects can't include  line endings unless the actual result includes line endings.  Really, I would like to just turn off line length testing in anything in he testing tree. Also there should be a decision about whether @covers is required. It's not present in the majority of tests and it would need a special sniff to only apply to tests.

Also (related to the other thread) we need to decide

protected function setUp ()

{

}

or 

protected function setUp ()

{

}

Since it is inconsistent.

Those were the most obvious issues looking through the codesniff results and the tree itself. 

Elin 

[Ian]

On Saturday, 27 October 2012 05:48:13 UTC-4, Elin Waring wrote:

In terms of cleaning up the tree .. we really need some sniffs specifically for that tree if it is going to work. For example, the biggest issue I see is line length. Tests have a lot of issues with line length because the expects can't include  line endings unless the actual result includes line endings.  Really, I would like to just turn off line length testing in anything in he testing tree. Also there should be a decision about whether @covers is required. It's not present in the majority of tests and it would need a special sniff to only apply to tests.

Covers should not be required.

 

Also (related to the other thread) we need to decide

protected function setUp ()

{

}

or 

protected function setUp ()

{

}

Since it is inconsistent.

Empty setUp and tearDown methods should in general be removed.  Non empty setUp and tearDown methods should be sure to call parent in case we want to add pre and post processing stuff in the future.

Ian

[Elin Waring]

I think people (myself included ) just put them in because other tests have them. So if we get them all out we'll see fewere copy and paste instances.

Elin

[Donald Gilbert]

If no is opposed, I'll go through the legacy tree and remove all instances of empty setUp and tearDown. And if those methods aren't yet calling the parent methods, I'll add it.

[Elin Waring]

That would be great.  I've got a pull request for a bunch of things in the joomla test tree so I'll take care of this issue too.

Elin

[Michael Babker]

PHPUnit's skeleton generator adds in the @covers tags by default (at least it was on 3.6, I haven't run it on 3.7), so when I was writing tests, I'd leave those tags in there not thinking twice about it.  From the looks of it, if the tag is used, then the code coverage info is limited to only the method in the tag (http://www.phpunit.de/manual/current/en/appendixes.annotations.html#appendixes.annotations.covers). 

[Donald Gilbert]

Here's the PR for fully cleaning up the Legacy Unit Test Tree - https://github.com/joomla/joomla-platform/pull/1653