Test cases for TestDefaultDeviceCodeService

[Sangeeta19]

No description provided.

[jricher]

Looks good so far, looking to see the rest of the pieces filled out.

[codecov-io]

Codecov Report

Merging #1368 into master will increase coverage by 0.7%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master    #1368     +/-   ##
===========================================
+ Coverage     24.67%   25.37%   +0.7%     
- Complexity      888      914     +26     
===========================================
  Files           209      209             
  Lines         11673    11676      +3     
  Branches       2116     2116             
===========================================
+ Hits           2880     2963     +83     
+ Misses         8287     8199     -88     
- Partials        506      514      +8

Impacted Files	Coverage Δ	Complexity Δ
...mitre/openid/connect/config/JsonMessageSource.java	83.87% <0%> (+83.87%)	16% <0%> (+16%)	⬆️
.../oauth2/service/impl/DefaultDeviceCodeService.java	96.87% <0%> (+96.87%)	10% <0%> (+10%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2d94f4...c12f15c. Read the comment docs.

[jricher]

You're getting the test to pass here through some side effects. If the underlying paging system were to change, it might suddenly fail because of these side effects.

[Sangeeta19]

Justin,
Instead of invoking Mockito.veriyf() with hard code values like 2 and 4, invoking with size of set expiredDeviceCodes1 will solve the issue.

Is my understanding of the your comment is correct?

[Sangeeta19]

will the below code snippet works in case if it is all about size:

Set expiredDeviceCodes1 = Sets.newHashSet(deviceCode1,deviceCode2);
Set expiredDeviceCodes2 = Sets.newHashSet();
Mockito.when(repository.getExpiredCodes())
.thenReturn(expiredDeviceCodes1,expiredDeviceCodes2);
service.clearExpiredDeviceCodes();

Mockito.verify(repository,times(expiredDeviceCodes1.size()))
.remove(Matchers.any(DeviceCode.class));
Mockito.verify(repository,times(expiredDeviceCodes1.size())).getExpiredCodes();

[jricher]

It's not about size, it's about how you're making your repository respond. The size calls are an improvement, but that's not the main issue. You're having your mocked repository return a full list twice and an empty list regardless of whether or not the expirations have been performed on it in the mean time. In other words, it's passing because the service thinks it's doing the right thing but it would still pass if something else a little weird happened.

[Sangeeta19]

By adding Sysout to method fetchPage() and doOperation(DeviceCode item) in implementation class, realized that for each call of fetchPage() : doOperation(DeviceCode item) calls are dependent on size of expiredCodesList, so all the items are removed.

Then after fetchPage() is called again , now empty set will be returned (from mocked repository) as it got emptied out in the last call.

This forced me to mock repository with list with elements for first call and zero elements in second call.

[jricher]

But that's only true if things are emptied out as expected with each page.

[jricher]

build still failing and the side effect of the clear tests hasn't been fixed

[Sangeeta19]

Justin, I tried and I could not come up with the better way of mocking the repository to test the clearExpiredDeviceCodes(). Could you please help me.