Community
Participate
Working Groups
The bundle "Developer Statistics" need some unit tests, which checks the internal logic. I have looked, which classes could be covered through some unit tests. The list see here: * TriggeredCommandCollector * ExtensionPointReader * DateFormatter * TableSorters For this class we have to create some unit tests.
After the unit tests we could create also some UI tests. Before we start with the UI tests, we have to check out SWTBot and the build integration with maven (include SWTBot to our automated maven build). If the integration will be successful finished, we could write some SWTBot tests, which checks for example the internal logic of the TableViewers (loading data etc.). If we have done this step, please contact me again.
The associated test plugin: o.e.r.stats.rcp.tests
(In reply to Timur Achmetow from comment #0) > The bundle "Developer Statistics" need some unit tests, which checks the > internal logic. > I have looked, which classes could be covered through some unit tests. The > list see here: > > * TriggeredCommandCollector > * ExtensionPointReader > * DateFormatter > * TableSorters > > For this class we have to create some unit tests. That sounds good, I will try this in the next few days.
Created attachment 240697 [details] Test class for DateFormatter I have created some small unit tests for the class DateFormatter.
(In reply to comment #3) > That sounds good, I will try this in the next few days. I have created a small example for the unit tests, see attachment.
(In reply to Timur Achmetow from comment #5) > (In reply to comment #3) > > That sounds good, I will try this in the next few days. > > I have created a small example for the unit tests, see attachment. Timur, can you also guide Akif through the whole Gerrit process please. Best to get prospective students acquainted with that as quickly as possible. Thanks.
(In reply to comment #6) > Timur, can you also guide Akif through the whole Gerrit process please. Best to > get prospective students acquainted with that as quickly as possible. Thanks. Yes of course, Akif have at the moment some problems with Gerrit. He will try to fix the problem.
(In reply to Timur Achmetow from comment #7) > Yes of course, Akif have at the moment some problems with Gerrit. He will > try to fix the problem. Yes that's right, I give my best.
Created attachment 240731 [details] New Version of Test
Created attachment 240732 [details] Extended Version of Formatter
(In reply to comment #7) > Yes of course, Akif have at the moment some problems with Gerrit. He will try to fix the problem. After 2 hours fighting against the CLA (problems with the login, web interface), now it works :D I have pushed my first change to Gerrit: https://git.eclipse.org/r/#/c/23167/
(In reply to Akif Eddy from comment #11) > After 2 hours fighting against the CLA (problems with the login, web > interface), now it works :D > I have pushed my first change to Gerrit: https://git.eclipse.org/r/#/c/23167/ Awesome. Thanks Akif. We'll move discussion of the change comment over to Gerrit now to discuss the actual code.
I have created several tests for DateFormatter and a few for TableSorters (see at gerrit). The TableSorters tests are JUnit-Plugin-Tests. Is that okay for our build server?
Yes, JUnit Plugin tests are fine for our build server. BTW, you can see what it does here: <https://hudson.eclipse.org/recommenders/job/stats.gerrit.master/>. Just click on "Console Output" for the build you are interested in.
(In reply to Andreas Sewe from comment #14) > Yes, JUnit Plugin tests are fine for our build server. Okay, that's great. Thank you.
The tests for the class "DateFormatter" are finished and merged, see here: https://git.eclipse.org/r/#/c/23167 Next test class: TableSorters.
New gerrit review see here: https://git.eclipse.org/r/#/c/23755/
@Tim: Have you a favorite for the next class?
I have create a first test for the class ExtensionPointReader, see here: https://git.eclipse.org/r/#/c/24412/
Test for ExtensionPointerReader merged.
Next candidate for our tests: TriggeredCommandCollector
(In reply to comment #21) > Next candidate for our tests: TriggeredCommandCollector Okay, I had a look on the class TriggeredCommandCollector and the logic of it. We have here the same problem like in the class ExtensionPointReader. The class is at the moment not good testable. We have to refactor it a little bit for better testing. I would move the two methods TriggeredCommandsView#loadModelFromWorkspace() and TriggeredCommandsView#buildViewerModel() in the class TriggeredCommandCollector and make them "protected". Then we could test this both methods with unit tests. Another point would be the collecting of events from the workbench, see TriggeredCommandCollector.listener (IExecutionListener) @Andreas: Is this testable with mockito btw. make it sense to test this (catching events and persist them)?
(In reply to comment #22) > I would move the two methods TriggeredCommandsView#loadModelFromWorkspace() and > TriggeredCommandsView#buildViewerModel() > in the class TriggeredCommandCollector and make them "protected". Then we could > test this both methods with unit tests. Change for this stuff is online: https://git.eclipse.org/r/25292
(In reply to Timur Achmetow from comment #23) > Change for this stuff is online: https://git.eclipse.org/r/25292 Great. I would continue the work on this change.
(In reply to Timur Achmetow from comment #22) > Another point would be the collecting of events from the workbench, see > TriggeredCommandCollector.listener (IExecutionListener) > @Andreas: Is this testable with mockito btw. make it sense to test this > (catching events and persist them)? It's certainly worth testing, yes. I wouldn't test the TriggeredCommandCollector itself (too much dependence on static Platform* methods and very little code), though, but the IExecutionListener. Move the IExecutionListener to its own class and test that by calling preExecute from you test code. To ease testing, I would also refrain from hard-coding the storage location in the IExecutionListener. At the very least, pass in a File to its constructor. Even better, though, would be to pass in a new StatisticsService that's responsible for persisting stuff on disk. The current design (one class writes to a file, another class reads from it) is unfortunate (and not thread-safe).
(In reply to Andreas Sewe from comment #25) > It's certainly worth testing, yes. I wouldn't test the > TriggeredCommandCollector itself (too much dependence on static Platform* > methods and very little code), though, but the IExecutionListener. > > Move the IExecutionListener to its own class and test that by calling > preExecute from you test code. > > To ease testing, I would also refrain from hard-coding the storage location > in the IExecutionListener. At the very least, pass in a File to its > constructor. Even better, though, would be to pass in a new > StatisticsService that's responsible for persisting stuff on disk. The > current design (one class writes to a file, another class reads from it) is > unfortunate (and not thread-safe). Change is online: https://git.eclipse.org/r/#/c/25934/
Thanks for the whole work guys! Really great! Merged Changes: https://git.eclipse.org/r/#/c/23167/ https://git.eclipse.org/r/#/c/23755/ https://git.eclipse.org/r/#/c/24412/ https://git.eclipse.org/r/#/c/25292/ https://git.eclipse.org/r/#/c/25934/
