Skip to content

Tests improve coverage#7

Merged
jankapunkt merged 15 commits intomasterfrom
tests-improve-coverage
Feb 4, 2022
Merged

Tests improve coverage#7
jankapunkt merged 15 commits intomasterfrom
tests-improve-coverage

Conversation

@jankapunkt
Copy link
Copy Markdown
Member

@jankapunkt jankapunkt commented Jan 20, 2022 •
edited
Loading

Intends to improve test coverage to such an extend, that we can safely refactor code to full ES6 without regressions

Also found and fixed additional issues:

  • removed old unsused gulp files
  • bas64 support for EJSON added (uses the same package as React Native), also browser compatible
  • EJSON.equals fails to compare Array and Object if first param is Object and second one is Array this is actually an issue with the original EJSON code and I already filed a bug to the Meteor project
  • added unit tests for selector.compileSelector and selector.compileSort
  • compileSelector used in $where a fallback if argument is not a function, that relied on the Function constructor, which is replaced now with an arrow function closure
  • added unit tests for all transaction types

What remains to be done but is out of scope for this PR:

  • testing integration with RN and RN-Meteor
  • moving all code to es6; remove decaffeinate autocode (suggested by commets)
  • update lodash and eventemitter3 (vulneraibilites!) but that requires careful crafting as they have partially different apis (two/three major version differences)
  • add GitHub CodeQL to CI to find potential vulnerable code definitions not picked up by tests or linter

current coverage (updated on each push):

--------------------------------|---------|----------|---------|---------|---------------------
File                            | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s   
--------------------------------|---------|----------|---------|---------|---------------------
All files                       |   91.86 |    90.73 |   94.11 |   92.04 |                     
 EJSON.js                       |   98.91 |    98.49 |     100 |     100 | 140,190             
 MemoryDb.js                    |   94.93 |    88.23 |      90 |    94.8 | 32-36,59,68         
 NullTransaction.js             |     100 |      100 |     100 |     100 |                     
 ReadOnlyTransaction.js         |     100 |      100 |     100 |     100 |                     
 ReadTransaction.js             |     100 |      100 |     100 |     100 |                     
 SynchronousWriteTransaction.js |     100 |      100 |     100 |     100 |                     
 WithObservableReads.js         |   92.85 |    83.33 |   85.71 |   92.85 | 30,60-61            
 WithObservableWrites.js        |     100 |      100 |     100 |     100 |                     
 WithReactMixin.js              |    9.52 |        0 |       0 |    9.52 | 11-81,89-92         
 WithServerQuery.js             |   85.93 |       80 |   78.57 |   85.93 | 26,117-126          
 WriteTransaction.js            |   89.28 |    83.33 |     100 |   89.09 | 86-87,92-96         
 selector.js                    |   96.52 |     92.1 |     100 |   97.84 | 417,421-423,495,659 
 utils.js                       |   96.09 |    91.89 |     100 |   95.93 | 161,214,225,252,300 
--------------------------------|---------|----------|---------|---------|---------------------

@jankapunkt jankapunkt linked an issue Jan 20, 2022 that may be closed by this pull request
Restructure and improve Tests #5
Closed
@jankapunkt jankapunkt marked this pull request as ready for review January 21, 2022 23:05
@jankapunkt jankapunkt requested a review from not-ai-yet January 21, 2022 23:05
@jankapunkt
Copy link
Copy Markdown
Member Author

jankapunkt commented Feb 3, 2022

@TheRealNate did you get a chance to review this one?

@jankapunkt jankapunkt requested a review from diavrank February 3, 2022 08:20
not-ai-yet
not-ai-yet approved these changes Feb 3, 2022
View reviewed changes
Copy link
Copy Markdown

@not-ai-yet not-ai-yet left a comment

Choose a reason for hiding this comment

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

Hey @jankapunkt, sorry about the delay. This is clearly a lot of work! These look good to me, feel free to merge whenever erady.

@jankapunkt jankapunkt merged commit 1425623 into master Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@not-ai-yet not-ai-yet not-ai-yet approved these changes

@diavrank diavrank Awaiting requested review from diavrank

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Restructure and improve Tests

2 participants

@jankapunkt @not-ai-yet