Re: D19 Comments and Diff

2014-03-06 Thread Kamil Paral
  I also want to avoid increasing the maintenance burden of having a
  bunch of tests that look for things which don't really need to be
  tested (setting data members, checking default values etc.)
 
 I agree, there is stuff that kind of can be taken for granted.

Coincidentally, I have an anecdote to tell... of this morning.

I have went ahead and implemented Tim's suggestion here:
https://phab.qadevel.cloud.fedoraproject.org/D19?id=49#inline-127

A damn simple change, just replace (slightly modified for readability):
self._result = None
with
self._result = 'NEEDS_INSPECTION'
in the constructor and a trivial adjustment in the result getter.

However, my 'trivial' tests actually saved me:

=== FAILURES ===
___ TestCheckDetail.test_init_raise 

self = testing.test_check.TestCheckDetail instance at 0x2fc1d88

def test_init_raise(self):
'''Test instance creation that raises an error - invalid parameters'''
with pytest.raises(exc.TaskotronValueError):
   check.CheckDetail(result='INVALID TYPE')
E   Failed: DID NOT RAISE

testing/test_check.py:38: Failed
__ TestCheckDetail.test_update_result __

self = testing.test_check.TestCheckDetail instance at 0x343f638

def test_update_result(self):
'''Test 'update_result' method'''
# basic use case
self.cd.update_result('FAILED')
   assert self.cd.result == 'FAILED'
E   assert 'NEEDS_INSPECTION' == 'FAILED'
E - NEEDS_INSPECTION
E + FAILED

testing/test_check.py:66: AssertionError
 2 failed, 50 passed, 1 skipped in 0.32 seconds 

So, even those simple tests have some value, sometimes.

Of course, there's no point in testing whether simple class.attr = value 
statement really works. But if attr is a property (having setter/getter), now 
it starts to make sense. And also if the documentation says it should have some 
default value (other than None), it's useful to test it. I don't think it's 
strictly required, but I definitely wouldn't call it unnecessary.
___
qa-devel mailing list
qa-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/qa-devel


Re: D19 Comments and Diff

2014-03-06 Thread Kamil Paral
 That being said, I don't really want to start off with tests that are
 putting too many asserts into the one test. Some combinations are fine
 but I'd prefer erring on the side of separating things out a bit too
 much for now rather than risk setting an example of large, monolithic
 tests.

I agree, and it's especially important for more complex code. But in this case, 
I think I've hit the sweet spot. The tests are really simple, and therefore 
they can be clustered together (in a reasonable extent) for improved 
readability and reduced test code length.

By the way, I've pushed some more revisions of the patch. It contains 64 lines 
of code + 144 lines of documentation and whitespace, and 208 lines of code of 
tests. Pylint score is 100% and test coverage is 100%. I think it's my best 
code ever, by all metrics ;) Let's not be too nitpicky here.
___
qa-devel mailing list
qa-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/qa-devel


Re: Default invocation of pytest for qadevel projects

2014-03-06 Thread Tim Flink
On Thu, 6 Mar 2014 05:44:15 -0500 (EST)
Kamil Paral kpa...@redhat.com wrote:

  For the qadevel projects I'm aware of, we've been using pytest for
  our unit/functional test runner.
  
  As part of the shared configuration, tests are split up into two
  categories, unit and functional. Unit tests are fast, do not touch
  the network, database or filesystem (there are some exceptions to
  that last part, though). Functional tests tend to be more
  integration tests that can set up a database or do other actions
  which fall outside of the previous definition of unit test.
  
  When you run pytest without any extra args, only the unit tests are
  run. The '--functional' arg is needed to enable collection and
  execution of the functional tests.
  
  Kamil recently made a request [1] to do one of two things:
  
  [1] https://phab.qadevel.cloud.fedoraproject.org/T89
  
  1. Change the default such that functional tests are collected and
  exclude them from collection using a '--unit' arg
  
  2. Change the functional arg from '--functional' to something
  shorter, like '--func'
  
* note that there are restrictions on which args we can use. For
  example, '-f' is not allowed as single letter args are reserved
  for pytest itself
  
  As stated in T89, I don't have a strong opinion on this as long as
  it's possible to exclude the functional tests from collection and
  we make the same change across all of our active projects. However,
  I wanted to put this up for a wider discussion before changing
  things.
  
  Any other thoughts on this proposed change?
 
 I should remember to read the mailing list before the Phab comments.
 I already closed the task as wontfix with my comment included:
 https://phab.qadevel.cloud.fedoraproject.org/T89#9
 
 I agree that you have a point in running just the unit tests by
 default.

For my workflow, sure. On the other hand, I'm not sure what other
peoples' workflow looks like and if it reduces confusion, we may be
better off changing the defaults to run functional tests. As I stated
in the ticket, as long as I can turn off the functional tests with a
command line arg, my workflow won't be affected.

Other thoughts on which is a better default?

 Changing --functional to --func would be nice, though. It
 even matches the file prefix for func tests, 'functest_'.

Sure, I'm game for changing that. The two args that come to mind are:

 * --long (more generic than functional)
 * --func (short for functional)

Any thoughts on which of those (if either) would be better?

Tim


signature.asc
Description: PGP signature
___
qa-devel mailing list
qa-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/qa-devel


Re: Default invocation of pytest for qadevel projects

2014-03-06 Thread Josef Skladanka
 Any thoughts on which of those (if either) would be better?

I do not really mind either, and do not have any strong preference. I'm used to 
having the non-functional tests run by default, but I can easily manage any way 
we decide to do it.

j.
___
qa-devel mailing list
qa-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/qa-devel


Re: Default invocation of pytest for qadevel projects

2014-03-06 Thread Martin Krizek
- Original Message -
 From: Tim Flink tfl...@redhat.com
 To: qa-devel@lists.fedoraproject.org
 Sent: Thursday, March 6, 2014 3:29:32 PM
 Subject: Re: Default invocation of pytest for qadevel projects
 
 On Thu, 6 Mar 2014 05:44:15 -0500 (EST)
 Kamil Paral kpa...@redhat.com wrote:
 
   For the qadevel projects I'm aware of, we've been using pytest for
   our unit/functional test runner.
   
   As part of the shared configuration, tests are split up into two
   categories, unit and functional. Unit tests are fast, do not touch
   the network, database or filesystem (there are some exceptions to
   that last part, though). Functional tests tend to be more
   integration tests that can set up a database or do other actions
   which fall outside of the previous definition of unit test.
   
   When you run pytest without any extra args, only the unit tests are
   run. The '--functional' arg is needed to enable collection and
   execution of the functional tests.
   
   Kamil recently made a request [1] to do one of two things:
   
   [1] https://phab.qadevel.cloud.fedoraproject.org/T89
   
   1. Change the default such that functional tests are collected and
   exclude them from collection using a '--unit' arg
   
   2. Change the functional arg from '--functional' to something
   shorter, like '--func'
   
 * note that there are restrictions on which args we can use. For
   example, '-f' is not allowed as single letter args are reserved
   for pytest itself
   
   As stated in T89, I don't have a strong opinion on this as long as
   it's possible to exclude the functional tests from collection and
   we make the same change across all of our active projects. However,
   I wanted to put this up for a wider discussion before changing
   things.
   
   Any other thoughts on this proposed change?
  
  I should remember to read the mailing list before the Phab comments.
  I already closed the task as wontfix with my comment included:
  https://phab.qadevel.cloud.fedoraproject.org/T89#9
  
  I agree that you have a point in running just the unit tests by
  default.
 
 For my workflow, sure. On the other hand, I'm not sure what other
 peoples' workflow looks like and if it reduces confusion, we may be
 better off changing the defaults to run functional tests. As I stated
 in the ticket, as long as I can turn off the functional tests with a
 command line arg, my workflow won't be affected.
 
 Other thoughts on which is a better default?
 

I don't have a strong feeling about either but I'd run all the tests by default 
if I had to choose (given that functional ones don't run forever, which is 
not the case at this point).

  Changing --functional to --func would be nice, though. It
  even matches the file prefix for func tests, 'functest_'.
 
 Sure, I'm game for changing that. The two args that come to mind are:
 
  * --long (more generic than functional)
  * --func (short for functional)
 
 Any thoughts on which of those (if either) would be better?
 

--func sounds better to me.

Martin
___
qa-devel mailing list
qa-devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/qa-devel