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 <[email protected]
<mailto:[email protected]>> 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 <[email protected]
<mailto:[email protected]>> 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 <[email protected]>
>> 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 <[email protected]
<mailto:[email protected]>> 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
<[email protected] <mailto:[email protected]>>
>> 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>
>>