Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add automated tests #48

Open
curioustechizen opened this issue Apr 27, 2017 · 4 comments
Open

Add automated tests #48

curioustechizen opened this issue Apr 27, 2017 · 4 comments

Comments

@curioustechizen
Copy link
Owner

Introduce an abstraction for getting time (instead of using System.currentTimeMillis()) directly in RTTV.

Build on top of this to add automated tests, especially around scheduling the UpdateTimeTask.

@kwak123
Copy link

kwak123 commented Feb 15, 2018

Hello,

I'm in the process of adding unit tests, but I'm running into a couple dependency problems. Adding some setters and hooking into the init seems a possible solution, but I wanted to run it by you first (and introduce myself I suppose).

For example, RTTV currently instantiates its own handler for use on line 190. Adding a setter to set a Mockito variant would be helpful for unit testing, plus having everything instantiate in init() would add some consistency

private void init(Context context, AttributeSet attrs) {
    ...
    
    if (mHandler == null) {
        mHandler = new Handler();
    }
}

=======================
EDIT:
There's an ongoing problem with tight coupling that makes testing difficult (impossible?) without an API change, e.g. Handler.post() invoked by init(), which is invoked in constructor. Consider moving setReferenceTime() to separate "start()" function?

@curioustechizen
Copy link
Owner Author

@kwak123 Thanks for taking a stab at this. I will not have time to look into it in detail for a couple of weeks though. That said, your inputs are appreciated and I will follow up on your comments when I have time.

@kwak123
Copy link

kwak123 commented Feb 21, 2018

No problem. If you're fine with it, I can open up a PR with a possible approach

@curioustechizen
Copy link
Owner Author

I'd prefer to discuss first before you put in the effort to make a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants