Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add test framework and basic tests #13

Merged
merged 13 commits into from Jun 26, 2017

Conversation

xcskier56
Copy link
Contributor

@xcskier56 xcskier56 commented Dec 17, 2016

The goal with this PR is to setup and add a testing framework using Testem and QUnit and add some basic tests. I'm happy to incorporate any feedback/modifications that you think are needed

Tests added:

  1. Basic test for init, it should add the expected style and ptr elements.
  2. Testing that the destroy() function removes the style and ptr elements.
  3. Init is idempotent and won't creating multiple style or ptr elements.

I'd like to somehow write some tests for the event handlers. My thought is to export the functions alongside init() so that I can test them without having to actually trigger events. @pateketrueke, do you have any ideas on how to do this?

@pateketrueke
Copy link
Collaborator

I would like to setup this on a CI service, do you have any suggestion?

@pateketrueke
Copy link
Collaborator

pateketrueke commented Dec 18, 2016

Oh yeah, CI is painful. 🍺

My first thoughts on testing interaction was "how simulating gestures?" but I think later, gestures provide input, so we need to simulate that input...

Lets say, giving Y scroll the min-height should be N according the options.

I don't know how to do this...

@xcskier56
Copy link
Contributor Author

Haha yeah. I've gotten a basic setup working on travis-ci, but I've run into some issue with PhantomJS like to working with ES6 and having oddities with some dom interactions. The real challenge will be getting good tests going.

As for how to test this, at first I went down a similar path to you, even trying to manually simulate touch events on the trigger element. I don't think that will be a fruitful path... far too difficult. I think you're totally right, we need to simulate the input.

What I have seen work in the past for me is moving towards a more functional style of programming. That way the core of the functionality would be pure functions, and become easier to separate and test. The basic idea would be to store all of the state in an object, that would include things like pullStartY, pullMoveY, and _state. Then the functions would be passed the state, the settings, and the event. They would then modify the state. Something like:

function _onTouchStart(e, currentState, settings) {
  // all of the functionality would go in here
  return { currentState: currentState, settings: settings }
}

function touchStartHandler(e) {
  { currentState, _SETTINGS as settings } = _onTouchStart(e, currentState, _SETTINGS)
}

window.addEventListener('touchstart', touchStartHandler);

With this setup, we could pass an mock for the event and simulate different states without having to use real touch events.

Unfortunately, I think that this would require a fairly large rewrite of the project. We'd have to really separate the state from the functionality, but the end result would be a modular, testable project.

As a way to start writing tests without having to really redo everything, what if we were to export the _onTouchStart functions so that the test setup has access to them and then change around the settings and state variables to test. That way we could call the functions directly, and observe the results.

What do you think?

@xcskier56
Copy link
Contributor Author

Ok, I've got travis CI working. In order to fix the ES6 issues, I've moved the test files to /src and am using tarima to transpile them and placed them into /tests. Then testem picks up the files from there and runs the tests.

At this point I'm reasonably happy with where this PR is at. The things that I'd like you to review is what I've done in _run. I've added some guard clauses to prevent creating multiple style and ptr elements.

@pateketrueke
Copy link
Collaborator

pateketrueke commented Dec 18, 2016

What you've done for CI is awesome, so much thanks!

The only thing I would change:

// somewhere at startup
const _STYLE = `_ptr_styles${new Date().getTime()}`;

// all nodes with unique "id" are globals,
// we can use them instead of querying:
if (typeof window[_STYLE] === 'undefined') {
  styleEl = document.createElement('style');
  styleEl.setAttribute('id', _STYLE);

  document.head.appendChild(styleEl);
} else {
  styleEl = window[_STYLE];
}

// we need to replace the css every time,
// this due classPrefix may vary:
styleEl.textContent = getStyles()
  .replace(/__PREFIX__/g, classPrefix)
  .replace(/\s+/g, ' ');

I like the idea of reusing nodes rather that adding and removing them later.

Each callback, e.g. _onTouchMove is reading from _SETTINGS on each tick.

If you change the classPrefix option on the fly it will produce undesired effects may be?

Multiple calls of init() without teardown could reuse the same injected node for stylesheets this way.

Of course if you call destroy() it should remove all created nodes as expected.

@xcskier56
Copy link
Contributor Author

@pateketrueke, apologies for letting this hang out. A couple of pressing issues have come up in our main application and implementing pull to refresh has been temporarily suspended. I'll hopefully be able to get to this sometime next week.

Happy holidays!

@pateketrueke pateketrueke merged commit 534de4e into BoxFactura:master Jun 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants