Skip to main content


Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » Eclipse SmartHome » Timer reschedule()(Contributor advice needed)
Timer reschedule() [message #1797772] Tue, 06 November 2018 09:55 Go to next message
Jon Evans is currently offline Jon EvansFriend
Messages: 3
Registered: July 2009
Junior Member
Hi

I have reported a bug with the implementation of Timer#reschedule().

The JavaDoc for Timer#reschedule() says that you can call reschedule() at any time, even on a terminated timer ("This can also be called after a timer has terminated, which will result in another execution of the same code."). In practice, you can only call reschedule() either before the timer fires, or from the body of the timer while it is executing. You can't call it after it has executed (i.e. after hasTerminated() returns true).

I would like to fix this.

The first thing I've found is that there are currently no unit tests for the org.eclipse.smarthome.model.script.actions.ScriptExecution class, which is the class containing the createTimer method used in rules files. So, I've written a unit test for the current behaviour. I then went further and fixed the issue, and added a test for that as well. In order to fix the issue I had to modify the TimerImpl class so that it could hold onto the JobDataMap in case it had to schedule a new job.

I'm looking for advice about contributing this, as the contributor guidelines say to ask here first.

* My commit adding the unit test
* My commit fixing the bug and adding a test for it

So the advice I'm soliciting is:

* Does the approach look good?
* I didn't find a good way to test the interaction with the Quartz scheduler, so the test creates a job which fires in 10 milliseconds, then calls Thread.sleep(30) to wait for it to run. I couldn't find a better way but I'm open to ideas.
* I can run my individual unit test from the Eclipse IDE and it passes. mvn test fails due to other tests not passing.
* Is there anything else I've missed?
* Does anyone know how to set up Eclipse IDE so that it will pull down source code for dependencies? It seems to be disabled at the moment (set up via the official setup guide) because it doesn't consider any of the projects to be Maven projects.
* Do I need to fix up anything before creating Pull Requests, or should I just create them so people can comment on GitHub?
Re: Timer reschedule() [message #1797830 is a reply to message #1797772] Wed, 07 November 2018 07:47 Go to previous messageGo to next message
Henning Treu is currently offline Henning TreuFriend
Messages: 44
Registered: April 2017
Member
Hi Jon,

thanks a lot for taking this on!

You had me at
Quote:
So, I've written a unit test for the current behaviour.


Awesome to add tests first!

Please just follow the contribution guidelines (like signing the ECA, adding singed-off-by footer to the commit) and create a pull request on github. Other contributors and committers will start the review process there and give comments on your code. If necessary we refine your PR together and finally merge it.

I'm happy to find your contribution on github. See you there.

Cheers
Henning
Re: Timer reschedule() [message #1797846 is a reply to message #1797830] Wed, 07 November 2018 11:55 Go to previous message
Jon Evans is currently offline Jon EvansFriend
Messages: 3
Registered: July 2009
Junior Member
Thanks Henning.

I've created PRs #6479 and #6480.
Previous Topic:Newbee - .gitignore is broken
Next Topic:Need help understanding the rules engine implementation
Goto Forum:
  


Current Time: Mon Feb 24 16:17:25 GMT 2020

Powered by FUDForum. Page generated in 0.01671 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software

Back to the top