Before commit 3a9e2fdef9316e24b52ce99ac355fc2b09786c72 test_testimage_install() was testing install packages in an runtime test image without using the package manager. After such commit now it test package installation without package manager and PACKAGE_FEED_URIS variable that is used for package managers. It doesn't make a lot of sense to have both test in the same unittest; one testing a feature that doesn't require the package manager in the target and the other that test a feature of the package manager in the target.
The issue here was introduced because the testing of the PACKAGE_FEED_URIS variable requires of two steps. First it needs to check that the value in config was correctly moved to the target, and second, needs to make sure that the repository feeds can be fetched accordingly. Unfortunately, the first part is done by oe-selftest and the second is a better fit for testimage. Now what was done here is to take advantage of test_image_install (in oe-selftest), which had a workaround to trigger selftest.py in meta-selftest (testimage). A dnf test for PACKAGE_FEED_URIS was added to selftest.py and the required variables were configured in test_image_install to make it work. This saved time, avoiding issuing testimage twice, but it is true that the previous test in selftest.py is not related to the added one, which as stated is a dnf repo test. Our possible options here are the following: 1. Keep the current state of things and reject this bug 2. Write new testimage and oe-selftest classes for the PACKAGE_FEED_URIS, properly separating different things. This has the advantage that is better design, but it gives us a heavy processing by issuing testimage twice, which is bad considering this is just a simple variable test. 3. Move the whole testing of PACKAGE_FEED_URIS to selftest only, and limit ourselves to test that the value of variable was correctly moved to target. In this case we won't test if the feeds actually work, only that the value is written to target, but in performance it would be way better. Considering that this is just a simple variable testing, and not that common, I believe it is not worth the preformance or design burdens to put the entire functionality in the test suite. I would like to go with option 3, testing just the basics of this variable and avoiding extra workload while running selftest.
I would prefer to keep on-image testing with dnf. Remote package feeds is a feature that is very important to many people, and there's been talk that in the future YP will provide some kind of 'official package feed' as well. Perhaps you can rename test_testimage_install to test_testimage_selftest, and re-purpose it to test various runtime things (that would go into meta-selftest/lib/oeqa/runtime/cases/selftest.py) that require special configuration, but are not necessarily related to each other?
I can understand that, but still I have my doubts. Renaming the test is not going to get rid of the issues with this bug, and even if it is not a big thing now, this could turn into a mess if we let more things inside this file. Not really sure if it is worth to test this in this way. Copying Richard, hoping we can get another opinion here.
You can split those several things each into their own function, and call them in order: prepare_testimage_install(): ... prepares the package install test prepare_testimage_remotednffeeds(): ... prepare the package feeds test test_testimage_selftest(): prepare_testimage_install() prepare_testimage_remotednffeeds() .... boot the image and run tests on it This is not too messy, is it? The real issue here is that we need to automatically (e.g. via oe-selftest) test things that: a) require special configuration in local.conf (or elsewhere) b) require making and booting an image and running tests on it c) without having to make and boot the image separately for each of the things that need to be tested, if they can be tested together via single image. I'd really like to figure out a solution for this scenario - how's the above?
(In reply to comment #2) > I would prefer to keep on-image testing with dnf. Remote package feeds is a > feature that is very important to many people, and there's been talk that in > the future YP will provide some kind of 'official package feed' as well. > I think these tests are covered with runtime testing suite and the dnf tests; runtime runs every time an image is created in the autobuilders. The runtime testing selftest was introduced because testexport feature was being broken over and over again; these tests were created with very few build time or none at all. The problem now is that runtime selftest was extended and has one of tests that consume most time within selftest. In my humble opinion runtime seltest needs to be used for critical features or things that keep being broken. Remember the idea behind selftest is to check features of the build system and sometimes this would be very expensive regarding resource consumption.
(In reply to comment #5) > (In reply to comment #2) > > I would prefer to keep on-image testing with dnf. Remote package feeds is a > > feature that is very important to many people, and there's been talk that in > > the future YP will provide some kind of 'official package feed' as well. > > > > I think these tests are covered with runtime testing suite and the dnf > tests; runtime runs every time an image is created in the autobuilders. Remote package feeds are not tested with dnf runtime tests. It is not possible to do, because the feed configuration needs to be set up in local.conf, which is only possible within selftest framework. > The runtime testing selftest was introduced because testexport feature was > being broken over and over again; these tests were created with very few > build time or none at all. The problem now is that runtime selftest was > extended and has one of tests that consume most time within selftest. In my > humble opinion runtime seltest needs to be used for critical features or > things that keep being broken. Does the remote feed test contribute a lot to time consumption? I don't see how; please explain. It's just booting an image (which would've been booted anyway for other testing purposes) and running a few commands on it. A few extra seconds at most. We need more testing, not less :) It's fine to strive for faster turnaround times, but not when you do it by removing useful tests!
Oh, and when support for signing remote package feeds is added, we *definitely* want to test that signature verification works, on the image (bug 11209). The existing test can be easily modified to accommodate that.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #2) > > > I would prefer to keep on-image testing with dnf. Remote package feeds is a > > > feature that is very important to many people, and there's been talk that in > > > the future YP will provide some kind of 'official package feed' as well. > > > > > > > I think these tests are covered with runtime testing suite and the dnf > > tests; runtime runs every time an image is created in the autobuilders. > > Remote package feeds are not tested with dnf runtime tests. It is not > possible to do, because the feed configuration needs to be set up in > local.conf, which is only possible within selftest framework. > > > The runtime testing selftest was introduced because testexport feature was > > being broken over and over again; these tests were created with very few > > build time or none at all. The problem now is that runtime selftest was > > extended and has one of tests that consume most time within selftest. In my > > humble opinion runtime seltest needs to be used for critical features or > > things that keep being broken. I wasn't referring to package feed test, I was referring to test_verify_postinst. > Does the remote feed test contribute a lot to time consumption? I don't see > how; please explain. It's just booting an image (which would've been booted > anyway for other testing purposes) and running a few commands on it. A few > extra seconds at most. I think this is the case when you boot a single image and do several tests in it. I don't think this is a bad approach to perform the test, but for reporting and history purposes it will be a mess; just imagine having 5 "sub-tests" in a single test and one of them failed. I think this is something that José would like to comment on. > > We need more testing, not less :) It's fine to strive for faster turnaround > times, but not when you do it by removing useful tests! I'm not saying to remove a test, just that we must be smart on them. selftest is becoming a pain point in the integration.
I discussed this with Jose just now. We definitely need to take the Dnf test outside of selftest.py, since those are different tests and doing so does not have any major implications. Now, the real issue is separating the calls to these tests in the test_testimage_install method. What I plan to do now is separating things inside that method there too, if shared states work there the time increase of doing two builds might not be that big. Will check times and comment back on that part.
(In reply to comment #9) > I discussed this with Jose just now. We definitely need to take the Dnf test > outside of selftest.py, since those are different tests and doing so does > not have any major implications. One of the most important principles of a good test case is that should be designed to be 'atomic', that is test only one thing at a time. As Mariano says adding more features to be tested will complicate failures tracking. > > Now, the real issue is separating the calls to these tests in the > test_testimage_install method. What I plan to do now is separating things > inside that method there too, if shared states work there the time increase > of doing two builds might not be that big. Will check times and comment back > on that part.
I'm fine with this FWIW.
Sent patch http://lists.openembedded.org/pipermail/openembedded-core/2017-June/137568.html
Fix in master http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=f1812e090b32f8c1bb33e4f26a246e5fb5e49c13