[keycloak-dev] Testing with mocking libraries?

[Stian Thorgersen]

On 7 June 2018 at 10:11, Steffen Kreutz <s.kreutz at yieldlab.de> wrote:

> I will remove the unit tests and mocking stuff from the PR and create
> integration tests.
>

Great, thanks


>
> 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.
>

Your code already checks if the hd param in the token matches the requested
hd though doesn't it? So testing valid/invalid domain seems sufficient to
me.


>
>
> > 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
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>