Re: [openstack-dev] [nova] proposal for unwinding database usage from tests

2015-01-28 Thread Chen CH Ji
Sorry for late reply and thanks for bring this out, I agree the create_db
flag will increase the complexity
so I might do some PoC and write a spec to do it next release

For this sentence, I don't fully understand, are you suggesting to every db
usage remove should be a
patch for a test class? thanks a lot
I'd like to propose instead DB usage should be removed per test Class as
an atomic unit.

Best Regards!

Kevin (Chen) Ji 纪 晨

Engineer, zVM Development, CSTL
Notes: Chen CH Ji/China/IBM@IBMCN   Internet: jiche...@cn.ibm.com
Phone: +86-10-82454158
Address: 3/F Ring Building, ZhongGuanCun Software Park, Haidian District,
Beijing 100193, PRC



From:   Sean Dague s...@dague.net
To: OpenStack Development Mailing List (not for usage questions)
openstack-dev@lists.openstack.org
Cc: Chen CH Ji/China/IBM@IBMCN
Date:   01/24/2015 07:13 PM
Subject:[nova] proposal for unwinding database usage from tests



I've been looking at the following patch series -
https://review.openstack.org/#/c/131691/13 for removing database
requirements from some tests.

I whole heartedly support getting DB usage out of tests, but I'd like to
make sure that we don't create new challenges in the process. The
conditional create_db parameter in test functions adds a bit more
internal test complexity than I think we should have.

I'd like to propose instead DB usage should be removed per test Class as
an atomic unit. If that turns into too large a patch that probably means
breaking apart the test class into smaller test classes first.

The other thing to be careful in understanding the results of timing
tests. The way the database fixture works it caches the migration
process -
https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L206

That actually means that the overhead of the db-migration sync is paid
only once per testr worker (it's 1s on my fast workstation, might be 2s
on gate nodes). The subsequence db setups for tests 2 - N in the worker
only take about 0.020s on my workstation (scale appropriately). So
removing all the unneeded db setup code is probably only going to save
~30s over an entire test run.

Which doesn't mean it shouldn't be done, there are other safety reasons
we shouldn't let every test randomly punch data into the db and still
pass. But time savings should not be the primary motivator here, because
it's actually not nearly as much gain as it looks like from running only
a small number of tests.

 -Sean

--
Sean Dague
http://dague.net

(See attached file: signature.asc)

signature.asc
Description: Binary data
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova] proposal for unwinding database usage from tests

2015-01-28 Thread Matt Riedemann



On 1/28/2015 10:59 AM, Chen CH Ji wrote:

Sorry for late reply and thanks for bring this out, I agree the
create_db flag will increase the complexity
so I might do some PoC and write a spec to do it next release

For this sentence, I don't fully understand, are you suggesting to every
db usage remove should be a
patch for a test class? thanks a lot
/I'd like to propose instead DB usage should be removed per test Class as
an atomic unit./

Best Regards!

Kevin (Chen) Ji 纪 晨

Engineer, zVM Development, CSTL
Notes: Chen CH Ji/China/IBM@IBMCN   Internet: jiche...@cn.ibm.com
Phone: +86-10-82454158
Address: 3/F Ring Building, ZhongGuanCun Software Park, Haidian
District, Beijing 100193, PRC

Inactive hide details for Sean Dague ---01/24/2015 07:13:45 PM---I've
been looking at the following patch series - https://reviSean Dague
---01/24/2015 07:13:45 PM---I've been looking at the following patch
series - https://review.openstack.org/#/c/131691/13 for rem

From: Sean Dague s...@dague.net
To: OpenStack Development Mailing List (not for usage questions)
openstack-dev@lists.openstack.org
Cc: Chen CH Ji/China/IBM@IBMCN
Date: 01/24/2015 07:13 PM
Subject: [nova] proposal for unwinding database usage from tests





I've been looking at the following patch series -
https://review.openstack.org/#/c/131691/13 for removing database
requirements from some tests.

I whole heartedly support getting DB usage out of tests, but I'd like to
make sure that we don't create new challenges in the process. The
conditional create_db parameter in test functions adds a bit more
internal test complexity than I think we should have.

I'd like to propose instead DB usage should be removed per test Class as
an atomic unit. If that turns into too large a patch that probably means
breaking apart the test class into smaller test classes first.

The other thing to be careful in understanding the results of timing
tests. The way the database fixture works it caches the migration
process -
https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L206

That actually means that the overhead of the db-migration sync is paid
only once per testr worker (it's 1s on my fast workstation, might be 2s
on gate nodes). The subsequence db setups for tests 2 - N in the worker
only take about 0.020s on my workstation (scale appropriately). So
removing all the unneeded db setup code is probably only going to save
~30s over an entire test run.

Which doesn't mean it shouldn't be done, there are other safety reasons
we shouldn't let every test randomly punch data into the db and still
pass. But time savings should not be the primary motivator here, because
it's actually not nearly as much gain as it looks like from running only
a small number of tests.

-Sean

--
Sean Dague
http://dague.net

/(See attached file: signature.asc)/


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



I take that to mean doing something like this:

https://review.openstack.org/#/c/142655/

But I don't want to speak for Sean.  But that would be a lot of work for 
some test classes, so I'd think it'd have to be phased, which is 
happening in some places, e.g. danpb was doing some of that with the 
libvirt unit test refactoring, and we've done some of that with the 
compute manager tests.  test_compute_mgr is huge though so refactoring 
and moving tests to NoDB will take time (test_neutronv2 also has some of 
this similar issue, i.e. we want to get away from the DB and mega-mox in 
setUp and move to using mock, so there is a separate test class in that 
module which uses Mock and NoDB and new tests should live there unless 
we can just update small parts of old tests).


So this is kind of like the mox-mock conversion, it's OK to update 
existing test cases for bug fixes, but if you have new unit tests (new 
test cases/classes), do them in mock (and do them w/o the DB).


--

Thanks,

Matt Riedemann


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova] proposal for unwinding database usage from tests

2015-01-28 Thread Chen CH Ji
ok, I understand the purpose and way to go now, thanks for the sharing
and I will refactory the patches I have ,thanks

Best Regards!

Kevin (Chen) Ji 纪 晨

Engineer, zVM Development, CSTL
Notes: Chen CH Ji/China/IBM@IBMCN   Internet: jiche...@cn.ibm.com
Phone: +86-10-82454158
Address: 3/F Ring Building, ZhongGuanCun Software Park, Haidian District,
Beijing 100193, PRC



From:   Matt Riedemann mrie...@linux.vnet.ibm.com
To: openstack-dev@lists.openstack.org
Date:   01/28/2015 06:17 PM
Subject:Re: [openstack-dev] [nova] proposal for unwinding database
usage from tests





On 1/28/2015 10:59 AM, Chen CH Ji wrote:
 Sorry for late reply and thanks for bring this out, I agree the
 create_db flag will increase the complexity
 so I might do some PoC and write a spec to do it next release

 For this sentence, I don't fully understand, are you suggesting to every
 db usage remove should be a
 patch for a test class? thanks a lot
 /I'd like to propose instead DB usage should be removed per test Class as
 an atomic unit./

 Best Regards!

 Kevin (Chen) Ji 纪 晨

 Engineer, zVM Development, CSTL
 Notes: Chen CH Ji/China/IBM@IBMCN   Internet: jiche...@cn.ibm.com
 Phone: +86-10-82454158
 Address: 3/F Ring Building, ZhongGuanCun Software Park, Haidian
 District, Beijing 100193, PRC

 Inactive hide details for Sean Dague ---01/24/2015 07:13:45 PM---I've
 been looking at the following patch series - https://reviSean Dague
 ---01/24/2015 07:13:45 PM---I've been looking at the following patch
 series - https://review.openstack.org/#/c/131691/13 for rem

 From: Sean Dague s...@dague.net
 To: OpenStack Development Mailing List (not for usage questions)
 openstack-dev@lists.openstack.org
 Cc: Chen CH Ji/China/IBM@IBMCN
 Date: 01/24/2015 07:13 PM
 Subject: [nova] proposal for unwinding database usage from tests

 



 I've been looking at the following patch series -
 https://review.openstack.org/#/c/131691/13 for removing database
 requirements from some tests.

 I whole heartedly support getting DB usage out of tests, but I'd like to
 make sure that we don't create new challenges in the process. The
 conditional create_db parameter in test functions adds a bit more
 internal test complexity than I think we should have.

 I'd like to propose instead DB usage should be removed per test Class as
 an atomic unit. If that turns into too large a patch that probably means
 breaking apart the test class into smaller test classes first.

 The other thing to be careful in understanding the results of timing
 tests. The way the database fixture works it caches the migration
 process -
 https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L206

 That actually means that the overhead of the db-migration sync is paid
 only once per testr worker (it's 1s on my fast workstation, might be 2s
 on gate nodes). The subsequence db setups for tests 2 - N in the worker
 only take about 0.020s on my workstation (scale appropriately). So
 removing all the unneeded db setup code is probably only going to save
 ~30s over an entire test run.

 Which doesn't mean it shouldn't be done, there are other safety reasons
 we shouldn't let every test randomly punch data into the db and still
 pass. But time savings should not be the primary motivator here, because
 it's actually not nearly as much gain as it looks like from running only
 a small number of tests.

 -Sean

 --
 Sean Dague
 http://dague.net

 /(See attached file: signature.asc)/



__
 OpenStack Development Mailing List (not for usage questions)
 Unsubscribe:
openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


I take that to mean doing something like this:

https://review.openstack.org/#/c/142655/

But I don't want to speak for Sean.  But that would be a lot of work for
some test classes, so I'd think it'd have to be phased, which is
happening in some places, e.g. danpb was doing some of that with the
libvirt unit test refactoring, and we've done some of that with the
compute manager tests.  test_compute_mgr is huge though so refactoring
and moving tests to NoDB will take time (test_neutronv2 also has some of
this similar issue, i.e. we want to get away from the DB and mega-mox in
setUp and move to using mock, so there is a separate test class in that
module which uses Mock and NoDB and new tests should live there unless
we can just update small parts of old tests).

So this is kind of like the mox-mock conversion, it's OK to update
existing test cases for bug fixes, but if you have new unit tests (new
test cases/classes), do them in mock (and do them w/o the DB).

--

Thanks,

Matt Riedemann


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe

Re: [openstack-dev] [nova] proposal for unwinding database usage from tests

2015-01-26 Thread Matt Riedemann



On 1/24/2015 12:13 PM, Sean Dague wrote:

I've been looking at the following patch series -
https://review.openstack.org/#/c/131691/13 for removing database
requirements from some tests.

I whole heartedly support getting DB usage out of tests, but I'd like to
make sure that we don't create new challenges in the process. The
conditional create_db parameter in test functions adds a bit more
internal test complexity than I think we should have.

I'd like to propose instead DB usage should be removed per test Class as
an atomic unit. If that turns into too large a patch that probably means
breaking apart the test class into smaller test classes first.

The other thing to be careful in understanding the results of timing
tests. The way the database fixture works it caches the migration
process -
https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L206

That actually means that the overhead of the db-migration sync is paid
only once per testr worker (it's 1s on my fast workstation, might be 2s
on gate nodes). The subsequence db setups for tests 2 - N in the worker
only take about 0.020s on my workstation (scale appropriately). So
removing all the unneeded db setup code is probably only going to save
~30s over an entire test run.

Which doesn't mean it shouldn't be done, there are other safety reasons
we shouldn't let every test randomly punch data into the db and still
pass. But time savings should not be the primary motivator here, because
it's actually not nearly as much gain as it looks like from running only
a small number of tests.

-Sean



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



Big +1 to doing this on a class by class basis, however, that can get 
ugly fast [1].


I commented on the change for test_compute to just move those to 
test_compute_api/mgr as appropriate, we should be freezing test_compute 
for the most part unless you're just modifying an existing test for some 
small wrinkle in the code.


I don't think we can completely change test_compute over to no-db in one 
change, that would be huge, we should just work on moving pieces over to 
those other modules that use NoDBTestCase (or move the bigger 
functional-type tests into nova.tests.functional).


One candidate is moving the big shelve/unshelve tests into 
test_compute_mgr/api, those have been a latent source of race bugs in 
the gate because of weird DB interactions/expectations/assumptions being 
borked by other tests running at the same time hitting the DB (because 
of so many different test classes that extend test_compute.BaseTestCase).


[1] https://review.openstack.org/#/c/142655/

--

Thanks,

Matt Riedemann


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [nova] proposal for unwinding database usage from tests

2015-01-24 Thread Mike Bayer


Sean Dague s...@dague.net wrote:

 I've been looking at the following patch series -
 https://review.openstack.org/#/c/131691/13 for removing database
 requirements from some tests.
 
 I whole heartedly support getting DB usage out of tests, but I'd like to
 make sure that we don't create new challenges in the process. The
 conditional create_db parameter in test functions adds a bit more
 internal test complexity than I think we should have.
 
 I'd like to propose instead DB usage should be removed per test Class as
 an atomic unit. If that turns into too large a patch that probably means
 breaking apart the test class into smaller test classes first.
 
 The other thing to be careful in understanding the results of timing
 tests. The way the database fixture works it caches the migration
 process -
 https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L206
 
 That actually means that the overhead of the db-migration sync is paid
 only once per testr worker (it's 1s on my fast workstation, might be 2s
 on gate nodes). The subsequence db setups for tests 2 - N in the worker
 only take about 0.020s on my workstation (scale appropriately). So
 removing all the unneeded db setup code is probably only going to save
 ~30s over an entire test run.
 
 Which doesn't mean it shouldn't be done, there are other safety reasons
 we shouldn't let every test randomly punch data into the db and still
 pass. But time savings should not be the primary motivator here, because
 it's actually not nearly as much gain as it looks like from running only
 a small number of tests.

I have a stalled patch for oslo.db that would provide for a new db fixture such 
that tests wouldn’t have to worry too much about when fixtures are set up or 
when per-test data is loaded; the system uses testresources to organize setup 
of schemas and teardown of data within longer-lived schemas in the most 
efficient way possible, across any number of backends:  
https://review.openstack.org/#/c/120870/.   Simplifying and bringing great 
consistency to the fixtures of participating projects and reducing the reliance 
upon SQLite in tests are the goals of this patch.  The patch is stalled not for 
any particular reason, except perhaps some lingering discomfort with the 
OptimisingTestSuite approach that necessitates patching into unittest internals 
in order to reorder whole groups of tests.

So yes, DB-specific tests and tests without DB absolutely have to remain broken 
up along class lines; the migration of an individual DB-test to a non-DB test 
should involve moving it to a different class that isn’t a DB-related fixture.  







 
   -Sean
 
 -- 
 Sean Dague
 http://dague.net
 
 __
 OpenStack Development Mailing List (not for usage questions)
 Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev