Testing for Comments/Annotation UX.

digi0ps commented 6 years ago

Apart from the unit tests (for functions, storage etc.), I have written a list of UI/behaviour entries, which if tests are added will make it much easier to find bugs. Since this is UX testing, we can go for headless browser testing. I have some experience with Casper, but I guess Jest + Puppeteer is popular. But I don't have much idea on how UX testing works with extensions, so what do you say @ShishKabab @poltak @oliversauter?

CommentBox

[x] Adding Comment
[x] Adding Comment with tags
[x] Adding Comments with tags overflow
[x] Comment box minimising
[x] Comment Box expanding

Annotation

[ ] Edit Annotation
[ ] Delete Annotation
[ ] Edit tags list
[ ] Tag holder
[ ] Go To Annotation (overview/in-page)

Sidebar

[x] Open sidebar
[ ] Open sidebar with highlights
[x] Close button
[x] Close sidebar when clicked outside
[x] Loading indicator

Tooltip

[ ] Create link
[ ] Create annotation
[ ] Open sidebar for annotation. Not for link.

(not a complete list)

poltak commented 6 years ago

Anyone have any ideas already for how the UI -> BG scripts separation will work?

An example: for "CommentBox - Adding Comment" flow, I can think of at least the following steps for how it roughly works in the browser (though not familiar with the actual implementation):

update the input's state
press the save button
send some data derived from the UI state over our remoteFunction messaging wrappers
FeatureStorage instance calls some StorageManager method to store the data
do something depending on outcome

1-3, 5 happen in the content script, 4 happens in the background script.

I feel the simplest way is just going to be mocking out step 4 (we just need to write a mock version of our util/webextensionRPC module), and covering the FeatureStorage class functionality with separate unit tests, rather than trying to get a web ext working in headless Chrome (some inconclusive discussions on that I was reading through: https://github.com/GoogleChrome/puppeteer/issues/659 https://github.com/GoogleChrome/puppeteer/issues/1215).

digi0ps commented 6 years ago

Yup that's nearly how the adding a comment/annotation flow work. But after storing the data in step 4, the annotations are fetched again from storage and reloaded in the UI.

https://developers.google.com/web/tools/puppeteer/examples There's an example which supports loading extensions using this way:

const browser = await puppeteer.launch({
  headless: false,
  args: [
    '--disable-extensions-except=/path/to/extension/',
    '--load-extension=/path/to/extension/',
  ]
});

If extensions don't work, then I would have to be mocking StorageManager too since IndexedDb for the background script won't be working, right?

poltak commented 6 years ago

If extensions don't work, then I would have to be mocking StorageManager too since IndexedDb for the background script won't be working, right?

The idea was more that you would just need to mock the background script as a whole (covered by just the util/webextensionRPC module). i.e., you test the stuff that happens in the content script separate to the background script. So for the content script stuff you just have to simulate successful and unsuccessful requests then deal with step 5 accordingly for both outcomes in your tests.

There's an example which supports loading extensions using this way:

That example seems to be running on a full version of Chromium rather than headless mode (header: false setting). What would that mean for situations like running the tests in our CI setup? I would be interested to learning more if you can get it running in puppeteer as a real multi-script extension though, but I'm apprehensive about the amount of effort that would be involved.

digi0ps commented 6 years ago

@poltak Got it setup and running yesterday. But there are a couple of caveats to this now.

First one, as you said, is that Chrome Extensions doesn't work in headless mode, so during every test Chromium pops up. As far as CI is concerned, Travis supports running GUI tests, but I maybe wrong.

Second one is that, I am not able to get the uglified css classnames from the stylesheets ( Proving to be a real pain, using selectors without classes right now ). Struggled a lot to solve this, but didn't work out. Importing .css files in jest fails (identity-obj-proxy or jest-css-modules doesn't help). If there is a way for me to access the uglified classnames mapping, it would solve this for me. Another solution I can think of is, disabling CSS class name uglification during UI testing alone, what do you say?

poltak commented 6 years ago

RE Chromium in CI: That Travis link talks about headless chrom/ium rather than the full thing; this is a really important distinction. If extensions don't work in headless mode but headless mode is required for our CI setup then there is a bit of a problem unless we can find an alternative way to get it running as an extension. If you have time, play around and see what you can get working (though if it proves to be a complete time waster, feel free to say so).

RE classnames: Yes, it is a real pain with the obfuscated classnames in UI dev right now. There's an issue for this here #418, so let's move that specific discussion there.

digi0ps commented 6 years ago

Okay I will dig deeper into it.

RE classnames: The obfuscation is not a problem right now. I need to be able to retrieve these transformed classnames through the tests, regardless of it being obfuscated or not. Like, in the puppeteer test, I want to fetch an element having some classname but I only have the original classname not the transformed. I want to able to get the transformed classname somehow. Do you get it?

ShishKabab commented 6 years ago

Re: remote functions. I remember creating a branch called 'headless-browser-tests' or something where I did mock out those.

The simplest for now I think would be indeed not to cover the extension API, and just do the integration tests by making some classes communicate directly.

As for browser testing, there are very few things that I think really depend on the browser. React testing (although a PITA) can be done without the browser using the Jest utilities, Enzyme or something like those: https://jestjs.io/docs/en/tutorial-react

What I can think of that might be useful is testing stuff with the real Dexie backend. But also there I remember putting in a mock somewhere. The only thing remaining then is the browser ext APIs, but seeing the abysmal docs there, I'd say to leave it for now.

digi0ps commented 6 years ago

Thanks for the info @ShishKabab. I got quite a few tests running in my ui-testing branch. But puppeteer doesn't allow an extension to be loaded in headless mode. So they fire a Chromium when running.

Right now, I am trying to add a new Jest setup for Puppeteer so that the each test suite doesn't spin up a new chrome. We can do the jest-react rendering tests with Enzyme, but we are testing a lot of User interactions (focussing, tooltip popup etc.) here for which I think Puppeteer will be a better choice.

WorldBrain / Memex