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

Add v6 tests (WIP) #528

Closed
wants to merge 6 commits into from
Closed

Add v6 tests (WIP) #528

wants to merge 6 commits into from

Conversation

timwis
Copy link
Member

@timwis timwis commented Jul 3, 2017

Not ready to merge yet, work in progress. Jotted down ideas for tests below. Let me know if any don't make sense, and feel free to add other ideas.

Unit test ideas

  • enables history by default
  • if history enabled, registers navigate, popstate, pushstate, replacestate events
  • enables href by default
  • if href enabled, clicking link trigers pushstate (failing, likely due to nanohref cancelling in node)
    • unless link is current location
  • .route registers the route
  • route is passed state and emit fn
    • verify emit fn context?
  • state is passed current route
  • state is passed current search query (jsdom doesn't support changing search)
  • callback passed to .use is passed state, emitter, and app
    • verify context?
  • on navigate event, emits render
  • on navigate event, updates query
  • on navigate event, scrolls to anchor
  • on popstate event, emits navigate
  • on pushstate event, modifies history and emits navigate
  • on replacestate event, replaces history and emits navigate
  • on window.onpopstate, emits popstate event
  • domcontentloaded event fired on document ready
  • mount morphs the target node
  • toString works
  • on render event, tree is morphed

higher level (closer to integration tests)

  • state change re-renders? (ie. choo 101 app)
  • * route called by default
    • /* route works too (maybe nanorouter)

also disable failing server rendering test and add failing nanohref test
@timwis timwis mentioned this pull request Jul 3, 2017
25 tasks
@yoshuawuyts
Copy link
Member

is passed state, emitter, and app

Oops, that thould not contain app haha

@yoshuawuyts
Copy link
Member

This is all great!

- route handler is passed state and emit
- state includes current route
- use is passed state and emitter
- state includes query
accidentally committed
var Nanobus = require('nanobus')
require('jsdom-global')(null, { url: 'http://localhost/' })
window.localStorage = window.localStorage || {} // See: yoshuawuyts/nanotiming#7
window.requestAnimationFrame = window.requestAnimationFrame || function (cb) { process.nextTick(cb) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a little tricky; perhaps using https://github.com/juliangruber/tape-run might make it easier by testing in an actual browser env?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh heck yeah, that'd make it easier. At least 1 other test simply wouldn't work otherwise too. Glad to hear you're good using that.

@yoshuawuyts
Copy link
Member

@timwis do you think this could be merged?

@goto-bus-stop
Copy link
Member

@timwis do you have time to rebase and merge soon? else would you mind if i do it? 😛

@timwis timwis mentioned this pull request Jul 15, 2018
@timwis timwis closed this Jul 15, 2018
@timwis
Copy link
Member Author

timwis commented Jul 15, 2018

I'm sooooo sorry I missed your comments! I was subscribed to all activity on the repo and got behind reading every single comment, so didn't realise I was mentioned here. I started to rebase this but decided to start over in another branch with hopefully better tests. Just submitted another PR.

goto-bus-stop pushed a commit that referenced this pull request Aug 14, 2018
Replaces #528. I was just going off the features listed in the readme. There are a handful more we should test but this is a start.

Note that this includes a readme change for `state.href` that I believe was incorrectly documented. Let me know if I got that wrong.

Also note that we should delete the original `v6-tests` branch once this is merged. Leaving it for now for reference.
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.

3 participants