meteorrn / meteor-react-native

Meteor client for React Native matching Meteor Spec
https://guide.meteor.com/react-native.html
Other
59 stars 31 forks source link

Implement tests + coverage #83

Closed jankapunkt closed 2 years ago

jankapunkt commented 2 years ago

Upgraded babel to v7 Upgrade react-native preset to module:metro-react-native-babel-preset Upgrade mocha/chai/sinon Added nyc for coverage + presets Updated ddp.js tests

Fixes #26

Note: since this required babel updates this needs to be tested for compatibility.

From here we can start implement all kinds of tests and also wirte intration tests for minimongo-cache which is about to be finished soon

Current coverage (will be updated with each push):

-------------------------|---------|----------|---------|---------|-----------------------------------------------------------
File                     | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                                         
-------------------------|---------|----------|---------|---------|-----------------------------------------------------------
All files                |   87.73 |    84.29 |   81.94 |    88.4 |                                                           
 helpers                 |   63.63 |      100 |   33.33 |      75 |                                                           
  reactNativeBindings.js |   63.63 |      100 |   33.33 |      75 | 14-15                                                     
 lib                     |   93.12 |    94.31 |   95.83 |   94.05 |                                                           
  Random.js              |     100 |      100 |     100 |     100 |                                                           
  ddp.js                 |   86.15 |     87.5 |   89.47 |   88.88 | 43-56                                                     
  mongo-id.js            |     100 |      100 |     100 |     100 |                                                           
  queue.js               |     100 |      100 |     100 |     100 |                                                           
  socket.js              |     100 |      100 |     100 |     100 |                                                           
  utils.js               |   83.33 |       88 |     100 |   83.33 | 38,71,77-78                                               
 src                     |   85.78 |    80.35 |   76.34 |    85.9 |                                                           
  Call.js                |     100 |       75 |     100 |     100 | 5                                                         
  Collection.js          |   88.59 |    81.25 |   78.12 |    89.9 | 98,215-230                                                
  Data.js                |   28.57 |    35.71 |   13.33 |   28.57 | 12-65,72-73,83-84                                         
  ReactiveDict.js        |     100 |      100 |     100 |     100 |                                                           
  Tracker.js             |   91.41 |    80.41 |   94.44 |   91.28 | 58-59,73-74,95-97,127,153,222,240,344,397,409,511,533,616 
-------------------------|---------|----------|---------|---------|-----------------------------------------------------------
TheRealNate commented 2 years ago

@jankapunkt re: compatibility, this only need to works on 0.60+. I'll run a test as soon as I can, but if you're able to do it sooner let me know.

jankapunkt commented 2 years ago

I have currently 0.64.3 installed and would use this for integration tests. Is that enough or should I test with another version as well?

TheRealNate commented 2 years ago

@jankapunkt that should be sufficient. Let me know if this is still WIP or you're ready for merging.

jankapunkt commented 2 years ago

@TheRealNate do I need specific bundling or something before I can test the package locally? Simply installing via npm + local path will throw errors like

./node_modules/@meteorrn/core/src/user/Accounts.js 8:16
Module parse failed: Unexpected token (8:16)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| 
| class AccountsPassword {
>   _hashPassword = hashPassword;
|   
|   createUser = (options, callback = () => {}) => {

I did not change or update this file and by looking at it it seems that some ESNext features are used but not transpiled correctly. How did you test the package locally last time?

Addition: I also found this to be an issue when importing Accounts.js to the tests, since the dependencies are not picked up by default by babel.

jankapunkt commented 2 years ago

Update: nevermind, I can acutally use npm pack and locally install the tar.gz to test everything. I will let you know when I tested it.

jankapunkt commented 2 years ago

I implemented the tests for /lib but there are still the tests missing for /src so this is still WIP. I also had the time to test with my RN dev env and it works fine so far. However I will test, after I have everything finish for merge, again.

jankapunkt commented 2 years ago

@TheRealNate I can't merge, can you do it please? Another question: can you make an NPM rc-release?

jankapunkt commented 2 years ago

@TheRealNate I'd like to ask, if you can lift my permissions? I would like to step up and keep this package maintained. As you might know, I am very active in the community and will give my best to keep the package well and up to date.

However, it would be much easier, if I can merge PRs or publish releases on GitHub and on NPM (I have 2FA enabled there). I could get some people from the community, that actually use this package in production, involved to make reviews. What do you think?

TheRealNate commented 2 years ago

@jankapunkt sorry about the delay. I've added you as a collaborator and am merging this PR.