Hi Rob!

Just checking into see if you had a chance to look into the unit testing issue at all, and if you had any findings you can share. I've been doing some stuff with Distill recently, and haven't made any more progress on this issue lately.

Let me know!

Josh

On 9/14/2019 1:29 AM, Joshua C. Poore wrote:

Thanks Rob,

Much appreciated. If all else fails we can build our own mock, and there are other npm packages that we can try. I started with dom-storage b/c its API suites our needs (has getItem, setItem), and chose v2.0.2 because its about as old as our version of jsdom :). We'll see what happens.

The sessionStorage thing is an important add for UserALE.js, it not only resolves some unwanted behavior in logging edge cases (e.g., form submit), but would allow for passing key:value data from sessionStorage to localStorage and thus provide a way to support apps that are designed for multi-tab use, by way of preserving sessionIDs. Its a big, little feature, if you get my meaning.

Thanks for the support!

Josh

On 9/13/2019 9:48 AM, Rob Foley wrote:

Seems you are doing the right thing (without having looked at the docs myself). I will try to look at it today/tonight. Not sure about the resource loader thing, could be that our use-case is just a non-issue in the new environment and thus is not possible.

Rob

On Fri, Sep 13, 2019 at 3:21 AM Joshua C. Poore <poor...@apache.org <mailto:poor...@apache.org>> wrote:

    Hey Rob,

    Completely agree. See comments in the ticket:
    https://issues.apache.org/jira/browse/FLAGON-451

    I found one npm package that looks promising for a storage mock:
    dom-storage (see ticket for links). I took a stab at it last
    night and
    the night before with very little luck. Pushed some of my work to
    Flagon-434 and documented findings in the ticket. Can you take a
    look--I
    don't think I'm passing the mocked storage into the jsdom env
    properly.
    I also played around with the getSessionId function so that it
    executes
    from a variable that's passed into getInitialSettings, rather than
    executed within getInitialSettings itself. Provides a bit better
    context
    in error messages. Its clear that jsdom still can't see the
    storage mock
    after passing it into the testing framework. Could really benefit
    from
    your expertise.

    Agree the test refactor can happen later. And agree that we
    should think
    about modernizing. I've been looking at Jest too. Seems a bit more
    intuitive and I'd love some nice tidy test coverage metrics. Also
    seems
    like we can preserve Chai tests, and Jest ships with jsdom, so
    maybe we
    could preserve a lot of our unit tests and just beef up testUtils. I
    wasn't clear on this: does Jest require that you create your own
    jsdom
    constructors, or have they abstracted it away in their own API? I
    HATE
    that new jsdom constructor, it drives me bonkers and I haven't
    figured
    out how we would pull our resource loader into their custom resource
    loader, yet (though, I'm still an amateur). Have tried and failed
    many,
    many times.

    Thanks for the help!

    Josh

    On 9/12/2019 9:42 AM, Rob Foley wrote:
    > So I think the answer here is to get a sessionStorage mock
    going. We can
    > either find one and add it to devDependencies or I can whip one up.
    > Upgrading the testing framework is probably worthwhile, but it
    may be out
    > of scope for this effort. We could investigate Jest, as I've
    had success
    > with that in the past. Either way, a big test refactor would be
    in order.
    >
    > Rob
    >
    > On Thu, Sep 12, 2019 at 1:24 AM Joshua Poore
    <poor...@apache.org <mailto:poor...@apache.org>> wrote:
    >
    >> @Rob
    >>
    >> Following up:
    >>
    >> I think the issue is that jsdom v9 has no concept of
    sessionStorage, so
    >> when getInitialSettings runs, we have a complete failure of
    UserALE.js
    >> within the jsdom environment. This would affect .options and
    .log APIs. I
    >> haven’t pushed more work on this, just have done some
    experimentation
    >> locally. But, if you re-work the getSessionId function to work
    outside of
    >> the getInitialSettings function and simply pass a variable
    into it, which
    >> is super ugly, you’ll get a new test failure error message
    indicating that
    >> sessionStorage is undefined. I didn’t push this because its
    not the best
    >> implementation that we should be working around. The reason
    why we’re not
    >> seeing the same error in the implementation on branch is
    because the
    >> getSessionId function is called from within
    getInitialSettings, so we see a
    >> report of .options being undefined.
    >>
    >> I’ve documented this here:
    >> https://issues.apache.org/jira/browse/FLAGON-451 <
    >> https://issues.apache.org/jira/browse/FLAGON-451>, which is
    where I’ve
    >> moved comments to from issue:
    >> https://issues.apache.org/jira/browse/FLAGON-441 <
    >> https://issues.apache.org/jira/browse/FLAGON-441>
    >>
    >> We have two options: there are some patchy things we can do
    with other
    >> modules (described in the ticket), which are still challenging
    given how we
    >> use jsdom.env, OR we can update our testing framework to jsdom v12
    >> (supports sessionStorage) which is also a lot of work.
    >>
    >> Rob (+ others) — I’m willing to put in some elbow grease on
    updating to
    >> jsdom v12. We may also need to update chai, but frankly our
    unit-testing
    >> framework is out of date and we’ll be faced with harder
    updates in the
    >> future. If I push some commits on this, will you be able to
    support?
    >>
    >> Josh
    >>
    >>> On Aug 12, 2019, at 2:21 PM, Joshua Poore
    <poor...@me.com.INVALID>
    >> wrote:
    >>> I did some more testing that I’ll document in the next few days.
    >>>
    >>> I think the issue is that JSDOM has no concept of
    window.sessionStorage.
    >> Which ever process calls the function I wrote breaks with a
    Type Error
    >> undefined, but if the function is pulled out of the context of
    >> getInitialSettings it looks like we get an error specific to
    >> sessionStorage. I’ve tried pulling in some add’l npm packages
    meant to
    >> define storage elements, but haven’t had success yet. Then
    again, haven’t
    >> had bandwidth to try really hard yet.
    >>> Thanks Rob. More findings soon.
    >>>
    >>> Joshua Poore
    >>>
    >>>
    >>>> On Aug 12, 2019, at 10:23 AM, Rob Foley <r...@apache.org
    <mailto:r...@apache.org>> wrote:
    >>>>
    >>>> I looked through the branch/issue and tried playing around
    with the
    >> code a
    >>>> bit. It seems like the userale script is not being added to
    the JSDom
    >>>> environment. Our custom resourceLoader (found in
    test/testUtils.js) is
    >>>> being ran, and is getting the correct content, however at
    some point
    >> JSDom
    >>>> is not actually putting that content into the page. I tried
    downgrading
    >>>> JSDom to an earlier version with no luck, so I am not sure
    if there is
    >>>> something else going on.
    >>>>
    >>>>> On Sat, Aug 10, 2019 at 12:47 AM Joshua Poore
    <poor...@apache.org <mailto:poor...@apache.org>>
    >> wrote:
    >>>>> Rob (or anyone else):
    >>>>>
    >>>>> any thoughts?
    >>>>>
    >>>>> issue: https://issues.apache.org/jira/browse/FLAGON-441 <
    >>>>> https://issues.apache.org/jira/browse/FLAGON-441>
    >>>>>
    >>>>> branch:
    >>>>>
    https://github.com/apache/incubator-flagon-useralejs/tree/FLAGON-434
    <
    >>>>>
    https://github.com/apache/incubator-flagon-useralejs/tree/FLAGON-434>
    >>

Reply via email to