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

UTs for HazelcastTimerStore #4311

Merged
merged 6 commits into from
Dec 12, 2019
Merged

Conversation

pzygielo
Copy link
Contributor

@pzygielo pzygielo commented Nov 9, 2019

Description

This isn't a bug fix / feature.

Testing

New tests

Check files changed tab.

Testing Performed

mvn clean install completed successfully

Test suites executed

All included in mvn clean install build.

Testing Environment

Ubuntu 16.04.6 LTS
OpenJDK Runtime Environment (build 1.8.0_222-8u222-b10-1ubuntu1~16.04.1-b10)
Maven 3.6.1

Documentation

Notes for Reviewers

Main change in production code: extracted old listTimers(String[] serverIds) to new pure function.

@AlanRoth AlanRoth added PR: CLA CLA submitted on PR by the contributor community-contribution labels Nov 11, 2019
@Cousjava
Copy link
Contributor

Jenkins test please

Copy link
Contributor

@jbee jbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pzygielo for having a look and adding some testing. As far as I am aware we do not use plain asserts so please use JUnit helper methods instead. There is the bigger question if instead of extracting a method to provide full generic type we add the generics to the field instead as it seems to be expected a fix type everywhere anyway. If you feel confident making that change please go ahead and do that.

@pzygielo
Copy link
Contributor Author

@jbee - thanks Jan for checking. I will rebase and update tests.
I'd be happy to check

if instead of extracting a method to provide full generic type we add the generics to the field instead as it seems to be expected a fix type everywhere anyway

in separate [at least] commit.

@pzygielo pzygielo requested a review from jbee November 27, 2019 13:08
@MarkWareham
Copy link
Contributor

Jenkins test please

@MarkWareham MarkWareham merged commit c290512 into payara:master Dec 12, 2019
@pzygielo pzygielo deleted the ut_for_store branch December 12, 2019 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: CLA CLA submitted on PR by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants