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

Implement tests + coverage #83

Merged
merged 4 commits into from
Apr 29, 2022

Conversation

jankapunkt
Copy link
Member

@jankapunkt jankapunkt commented Jan 22, 2022

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
Copy link
Collaborator

@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 jankapunkt mentioned this pull request Feb 8, 2022
@jankapunkt
Copy link
Member Author

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
Copy link
Collaborator

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

@jankapunkt
Copy link
Member Author

jankapunkt commented Feb 9, 2022

@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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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

@jankapunkt
Copy link
Member Author

@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 TheRealNate merged commit 0f00542 into meteorrn:master Apr 29, 2022
@TheRealNate
Copy link
Collaborator

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

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.

Help Wanted: Module Testing
2 participants