User tests: Successful: Unsuccessful:
This removes the check for the JForm object. If we don't use JForm in this class, we don't have to require it.
Labels |
Added:
?
|
Agreed on removing the JForm object check.
One rule for testing is that each test should test one thing, a good sign of breaking this rule is when you have more then one assertion in a test. As any rule there are exceptions but in this case you should write a test for each Exception. The object under test should be created in the setup method, this makes sure that each test runs under clean conditions and test two is not effected from the result of test one. You should also mock the input object because this test shouldn't test if JRegistry works. Same might be true for element but this is discussable. I would mock it either because we are not testing if SimpleXMLElement is working.
Forget the mention, we have some documentation about writing tests: https://docs.joomla.org/Unit_Testing_Best_Practices
Labels |
Added:
?
|
I updated the PR.
I have to say that I disagree with the rule "one assert per method". I test one method of a class per method of a unittest.
I have to say that I disagree with the rule "one assert per method". I test one method of a class per method of a unittest.
Then you are testing only if a method works or not and not what the conditions of the failure are. If you check what the experts on this area say then you will find that this rule is widely accepted.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-22 15:53:57 |
That looks much better! Thanks Hannes :)
Milestone |
Added: |
For the unit tests we should use the
@expectedException
tag and split the tests up. So we have one test for each exception etc. It's more maintainable long term (I know it's not how we're doing it in a lot of places - but that's why those tests are so hard to maintain)