Re: Review Request 119270: Autotest for corona restore and behavior
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62294 --- autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43276 That's not you. autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43277 that's frickin' clever! (not really a review comment I just wanted to say I thought it was nicely done) autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43278 Why? I thought the overriding the plugin loader was so we don't need real plugins. autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43279 why is this commented out? autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43280 leaks. autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43281 This will wipe the tester's settings, no? autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43282 maybe it's worth checking the sub-containments and applets immutiablity is propogated. - David Edmundson On July 14, 2014, 12:30 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 12:30 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
On July 14, 2014, 1:40 p.m., David Edmundson wrote: autotests/coronatest.cpp, lines 99-100 https://git.reviewboard.kde.org/r/119270/diff/1/?file=290064#file290064line99 This will wipe the tester's settings, no? yep, that's the intention, other tests do that as well: QStandardPaths::setTestModeEnabled(true); will make every path returned by qstandardpaths in ~/.qttests , so it ensures it doesn't touch the real system config (and ensures that the tests start from a blank space) - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62294 --- On July 14, 2014, 12:30 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 12:30 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
On July 14, 2014, 1:40 p.m., David Edmundson wrote: autotests/coronatest.cpp, line 70 https://git.reviewboard.kde.org/r/119270/diff/1/?file=290064#file290064line70 Why? I thought the overriding the plugin loader was so we don't need real plugins. whithout any sycoca built in the test directory it hits an assert: since all applet constructors call KService::serviceByStorageId(serviceID) even if serviceID is empty in this case, it will anyways bomb if there is no valid sycoca db.. - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62294 --- On July 14, 2014, 12:30 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 12:30 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
On July 14, 2014, 1:40 p.m., David Edmundson wrote: autotests/coronatest.cpp, line 81 https://git.reviewboard.kde.org/r/119270/diff/1/?file=290064#file290064line81 why is this commented out? copy and paste, i guess both using it a or not is fine, since it's waiting for the kbuildsycoca process to finish, but i reenabled it to be extra sure - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62294 --- On July 14, 2014, 12:30 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 12:30 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 5:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs (updated) - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62312 --- Ship it! autotests/coronatest.cpp https://git.reviewboard.kde.org/r/119270/#comment43294 it might make sense to make all of this in init()/cleanup() rather than initTestCase() that way each test gets it's own SimpleCorona and you don't have to worry about each test cleaning up after itself. - David Edmundson On July 14, 2014, 5:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 5:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
On July 14, 2014, 5:26 p.m., David Edmundson wrote: autotests/coronatest.cpp, line 95 https://git.reviewboard.kde.org/r/119270/diff/2/?file=290134#file290134line95 it might make sense to make all of this in init()/cleanup() rather than initTestCase() that way each test gets it's own SimpleCorona and you don't have to worry about each test cleaning up after itself. you mean calling it from each test or qtest does that automatically? - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62312 --- On July 14, 2014, 5:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 5:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
On July 14, 2014, 5:26 p.m., David Edmundson wrote: autotests/coronatest.cpp, line 95 https://git.reviewboard.kde.org/r/119270/diff/2/?file=290134#file290134line95 it might make sense to make all of this in init()/cleanup() rather than initTestCase() that way each test gets it's own SimpleCorona and you don't have to worry about each test cleaning up after itself. Marco Martin wrote: you mean calling it from each test or qtest does that automatically? QtTest does it automatically, it does: initTestCase() init() test1(); cleanup() init() test2() cleanup() ... cleanupTestCAse() - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62312 --- On July 14, 2014, 5:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 5:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
On July 14, 2014, 5:26 p.m., David Edmundson wrote: autotests/coronatest.cpp, line 95 https://git.reviewboard.kde.org/r/119270/diff/2/?file=290134#file290134line95 it might make sense to make all of this in init()/cleanup() rather than initTestCase() that way each test gets it's own SimpleCorona and you don't have to worry about each test cleaning up after itself. Marco Martin wrote: you mean calling it from each test or qtest does that automatically? David Edmundson wrote: QtTest does it automatically, it does: initTestCase() init() test1(); cleanup() init() test2() cleanup() ... cleanupTestCAse() hmm, the thing is that other tests would require the restore() and startupcompletition() to be ran before. i can move them inside init(), would lose a bit of granularity tough - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/#review62312 --- On July 14, 2014, 5:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 5:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119270: Autotest for corona restore and behavior
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119270/ --- (Updated July 14, 2014, 6:57 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- This is a simple autotest for loading a simple corona, checking that the uiready signals are emitted, that adding and removing applets work, and the immutability settings do work. The test is now passing and pretty basic, if someone can think about any new things to test, please add in the test ;) Diffs - autotests/CMakeLists.txt 060132d autotests/coronatest.h PRE-CREATION autotests/coronatest.cpp PRE-CREATION autotests/coronatestresources.qrc PRE-CREATION autotests/plasma-test-appletsrc PRE-CREATION src/plasma/corona.cpp e18f081 src/plasma/private/containment_p.cpp f8b4578 Diff: https://git.reviewboard.kde.org/r/119270/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
