JackAdams / meteor-transactions

App level transactions for Meteor + Mongo
http://transactions.taonova.com/
MIT License
113 stars 16 forks source link

Make symlink from test app to package relative so it works for everyone. #67

Closed mattmccutchen closed 7 years ago

mattmccutchen commented 7 years ago

Before this change, if /Users/babrahams/meteor-packages/transactions did not exist locally, then the test app would silently use the published version of the package.

With this change, I get four test failures:

FAILED jasmine-server-integration : rollback after updates are made => should still work with server error
/home/user/objsheets/tools/babrahams_transactions/testapp/.meteor/local/mirrors/jasmine-server-integration/.meteor/local/build/programs/server/app/tests/jasmine/server/integration/rollback-spec.js:130:59: Expected true to be false.

FAILED jasmine-server-integration : rollback after updates are made => should restore state if a non-existent document is removed
/home/user/objsheets/tools/babrahams_transactions/testapp/.meteor/local/mirrors/jasmine-server-integration/.meteor/local/build/programs/server/app/tests/jasmine/server/integration/rollback-spec.js:21:26: TypeError: Cannot read property 'transaction_id' of undefined

FAILED jasmine-server-integration : undo after multiple actions on a single doc field => should leave fields set that were already set
/home/user/objsheets/tools/babrahams_transactions/testapp/.meteor/local/mirrors/jasmine-server-integration/.meteor/local/build/programs/server/app/tests/jasmine/server/integration/undo-spec.js:21:26: TypeError: Cannot read property 'transaction_id' of undefined

FAILED jasmine-server-integration : undo after multiple actions on a single doc field => should return to initial state
/home/user/objsheets/tools/babrahams_transactions/testapp/.meteor/local/mirrors/jasmine-server-integration/.meteor/local/build/programs/server/app/tests/jasmine/server/integration/undo-spec.js:21:26: TypeError: Cannot read property 'transaction_id' of undefined

Here is the server log.

@JackAdams, can you reproduce this? Do you have an idea of what the problem is? If I revert bbdf3a6 and 2cdf02f, then the tests pass. If the test suite were working, I'd be happy to use it to test my changes before I send them to you, but I'm not willing to spend a lot of time debugging existing problems.

JackAdams commented 7 years ago

Yes, I've reproduced this. I think the reason for the bugs in the tests is that the relative symlink works for getting the package included in the velocity test reporter app, but the meteor mirrors that get started for running the tests don't have access to the right package location (or something like that).

I think the only quick fix is to do the absolute symlink, but that's a pretty rubbish solution. I'll have a poke around and see if I can find a better way.

JackAdams commented 7 years ago

I think I'm going to have to revert this back to an absolute symlink to make the tests run on the mirrors. Feel free to replace it with an absolute symlink of your own if you need to run the tests. I don't know enough about the (now unmaintained) Velocity testing framework to come up with a better alternative.

EDIT: nope, cancel that. I think these are pre-existing bugs I didn't know about. Doesn't seem to matter whether I use a relative or absolute symlink -- same four tests fail. Will investigate further.

JackAdams commented 7 years ago

Okay. Fixed those failing tests now. It was just me introducing a stupid bug in the last release and then failing to test against the new release. Works just fine with relative symlinking. Thank you again!

Let me release a new working version with your pull requests and #66 merged, then I'll get to work on the issue with the concurrent writes corrupting transactions.