comunica / sparqlee

⚙️ SPARQL expression evaluator library - Moved to @comunica/expression-evaluator
https://www.npmjs.com/package/sparqlee
14 stars 7 forks source link

Improves function tests code coverage #144

Closed Tpt closed 2 years ago

Tpt commented 2 years ago

All the lines in the functions directory should be covered except for the uplus function.

This PR also fixes float/doubles parsing and serialization of NaN and infinities.

It also removes some dead code in the helpers.

Tpt commented 2 years ago

About the "uplus" function, it is not emitted by the SPARQL parser (RubenVerborgh/SPARQL.js/issues/152). I see thre options to archive full code coverage:

  1. Drop it from sparqlee
  2. Emit it from SPARQL.js
  3. Add a "hand written" test to cover it.

@RubenVerborgh What do you think about it?

coveralls commented 2 years ago

Coverage Status

Coverage increased (+4.2%) to 95.867% when pulling e26fe083925c962a405cf8b40cbae06211168ef6 on Tpt:functions into 3c79552a6b8d1251a89a806b209185eb357ebf95 on comunica:master.

rubensworks commented 2 years ago

Perfect! No comments here :-)

About the "uplus" function, it is not emitted by the SPARQL parser (https://github.com/RubenVerborgh/SPARQL.js/issues/152). I see thre options to archive full code coverage:

If @RubenVerborgh agrees I would go for option 2: Emit it from SPARQL.js It might also require a small change in SPARQLAlgebra.js, but @joachimvh might know more about this.

joachimvh commented 2 years ago

It might also require a small change in SPARQLAlgebra.js, but @joachimvh might know more about this.

SPARQLAlgebra.js copies the expression functions directly from SPARQL.js so my guess is it would need no changes (except perhaps an update to the SPARQL.js dependency).

RubenVerborgh commented 2 years ago

If @RubenVerborgh agrees I would go for option 2: Emit it from SPARQL.js

Yes please!