Re: [openstack-dev] [nova] proposal for unwinding database usage from tests
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 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 > To: "OpenStack Development Mailing List (not for usage questions)" > > 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 th
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 To: "OpenStack Development Mailing List (not for usage questions)" 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
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 To: "OpenStack Development Mailing List (not for usage questions)" 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
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
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. 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
