[keycloak-dev] Testing with mocking libraries?

[Steffen Kreutz]

I updated the PR with the SocialLoginTest (https://github.com/keycloak/keycloak/pull/5215/files#diff-d1c89a9cccb653905cf757d2dcb67841 <https://github.com/keycloak/keycloak/pull/5215/files#diff-d1c89a9cccb653905cf757d2dcb67841>). Please tell me if this is done correctly.

Best,

Steffen

> Am 07.06.2018 um 10:11 schrieb Steffen Kreutz <s.kreutz at yieldlab.de>:
> 
> I will remove the unit tests and mocking stuff from the PR and create integration tests.
> 
> Would it be sufficient to test if logging in succeeds for an account with a valid domain and accordingly fails for an account with an invalid domain? Or should I somehow test if the token contains the hd param.
> 
> 
>> Am 06.06.2018 um 16:17 schrieb Marek Posolda <mposolda at redhat.com>:
>> 
>> +1 for investigate. I hope the contributor can help us with clarifying 
>> it and write the test?
>> 
>> Maybe the test for this will need to be in separate class, so it's 
>> possible to test it separately from the other Google IDP stuff, which 
>> can be tested with free account. And the other contributors to Google 
>> IDP, which don't have corporate account, won't be blocked.
>> 
>> But I suppose that for our CI, we can test with "redhat.com" domain account?
>> 
>> Marek
>> 
>> 
>> On 06/06/18 15:22, Stian Thorgersen wrote:
>>> I think we're in agreement here that the ideal is to do proper 
>>> integration tests. Rather than open the doors for mocks (java based or 
>>> http based) we should attempt to do it proper. For Google I'm not sure 
>>> how realistic that is as the hd param probably requires an corporate 
>>> account rather than a freebie account that we are using today for 
>>> testing. We should investigate that first though.
>>> 
>>> On 6 June 2018 at 10:03, Hynek Mlnarik <hmlnarik at redhat.com 
>>> <mailto:hmlnarik at redhat.com>> wrote:
>>> 
>>>   To answer the original question whether to mock or not to mock, I
>>>   wouldn't
>>>   object allowing those in unit tests (which we have not that many
>>>   though
>>>   until now there's no need to them and I hope we can preserve this
>>>   state).
>>>   I'd rather not introduce those in integration tests because of
>>>   maintenance
>>>   costs.
>>> 
>>>   The biggest issue I see with mocks is that one needs to be extra
>>>   careful to
>>>   return exactly what's expected from the specification/counterparty.
>>>   Since social
>>>   providers  can change without prior notice, our mocked IdPs might
>>>   give us
>>>   false impression that we're testing them. Furthermore, such mocks
>>>   would be
>>>   rather complex, and would require non-trivial maintenance. I'm
>>>   hence biased
>>>   against mock IdPs and prefer the tests with real IdPs as mentioned
>>>   by Marek.
>>> 
>>> 
>>>   On Mon, Jun 4, 2018 at 11:22 AM, Marek Posolda
>>>   <mposolda at redhat.com <mailto:mposolda at redhat.com>> wrote:
>>> 
>>>> Dne 1.6.2018 v 14:48 Bill Burke napsal(a):
>>>>> On Fri, Jun 1, 2018 at 5:18 AM, Marek Posolda
>>>   <mposolda at redhat.com <mailto:mposolda at redhat.com>>
>>>> wrote:
>>>>>> For IDP, I am not sure how would Mocked IDP help? We already
>>>   have lots
>>>> of
>>>>>> test coverage for Identity brokering for OIDC and SAML. The
>>>   social
>>>> providers
>>>>>> usually implements OAuth/OIDC and there are just small
>>>   differences
>>>> between
>>>>>> them. But those small differences and the fact that social
>>>   providers
>>>> are out
>>>>>> of our control, is exactly the reason why we need to test
>>>   with real
>>>>>> providers instead of mocks IMO.
>>>>>> 
>>>>> IDP mocks are not a replacement for real testing.  Just
>>>   something that
>>>>> would run in public CI, that way at least there's something to
>>>   catch
>>>>> issues like when/if brokering gets refactored again.
>>>> AFAIK the longer-term plan is to run social tests on every PR,
>>>   to catch
>>>> regressions by testing with real social providers. So maybe it's
>>>   better
>>>> to rather wait until we have that rather then introduce mocks now?
>>>> 
>>>> Also anyone from community should be able to run social tests
>>>   based on
>>>> our README. The important is, that someone could run it just for
>>>   single
>>>> provider, which he is interested in (EG. Google for this PR) and
>>>   is not
>>>> required to create the account+application in all social providers,
>>>> which we support. But I hope the social tests already support
>>>   this now.
>>>> 
>>>> Marek
>>>>> 
>>>>> Bill
>>>> 
>>>> 
>>>> _______________________________________________
>>>> keycloak-dev mailing list
>>>> keycloak-dev at lists.jboss.org <mailto:keycloak-dev at lists.jboss.org>
>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>   <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>>> 
>>> 
>>> 
>>> 
>>>   -- 
>>> 
>>>   --Hynek
>>>   _______________________________________________
>>>   keycloak-dev mailing list
>>>   keycloak-dev at lists.jboss.org <mailto:keycloak-dev at lists.jboss.org>
>>>   https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>   <https://lists.jboss.org/mailman/listinfo/keycloak-dev>
>>> 
>>> 
>> 
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>