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

Export type test helpers #163

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

bfirsh
Copy link
Contributor

@bfirsh bfirsh commented Jan 21, 2013

... so they can be used for testing custom types. See #156

@wmertens
Copy link
Contributor

I have no strong opinion - @josephg looks ok?

@bfirsh
Copy link
Contributor Author

bfirsh commented Apr 3, 2013

I've also included types.helpers here so bootstrapTransform can be used on the server. (It's only currently available in the browser as sharejs._bt).

@josephg
Copy link
Owner

josephg commented Apr 14, 2013

I'm confused - why is randomWord imported in each type's tests?

@josephg
Copy link
Owner

josephg commented Apr 14, 2013

This patch confuses me.

  • Most of the changes have nothing to do with exposing test helpers to external projects.
  • bootstrapTransform is already used by the server (the JSON and old text types use it).
  • Its bad form to expose test/ paths from the src/ directory. Do it at the top level instead, in src/index.coffee.

And also, the OT types have moved into their own project. Once you're happy with it, this pull request will need to be duplicated to josephg/ot-types .

@bfirsh
Copy link
Contributor Author

bfirsh commented Apr 15, 2013

Ahah – I didn't see the ot-types project before. That's pretty cool.

I moved randomWord into test/helpers/misc just for the sake of organising it. Would it be better in a separate file?

Would you prefer everything to be exposed as require('share').test?

@josephg
Copy link
Owner

josephg commented Apr 15, 2013

Yeah require('share').test sounds good. I moved randomWord in with the rest of the randomizer code in ot-types because you're right, it makes more sense like that.

Moving forward, I'm not really sure how these projects should be hooked up. I guess the ot types project should expose the randomizer and bootstrapTransform functions directly. If people want to use them from sharejs, they can just require('ot-types') directly.

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