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

test: add a test to verify point/pint compatibility #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ugzuzg
Copy link

@Ugzuzg Ugzuzg commented Nov 25, 2020

Trying to parse the pint unit before the point unit makes the point and meters incompatible.

For some reason, the signature of point and pint always matches and depends on the order they were parsed:

const Qty = require('js-quantities');

const point = new Qty('1point');
const pint = new Qty('1pint');
console.log(pint.signature, point.signature); // -> 1 1
const Qty = require('js-quantities');

const pint = new Qty('1pint');
const point = new Qty('1point');
console.log(pint.signature, point.signature); // -> 3 3

Linked to #26, I guess.

@Ugzuzg
Copy link
Author

Ugzuzg commented Jan 6, 2021

@gentooboontoo, I will gladly implement a fix if you can give me some directions.


it("should return true for mm and point", function() {
// if a pint was parsed before point
// point and meter should not become incompatible
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably due to both "pint" and "point" having "pt" as an alternative designation?

It might make sense to add a test that runs the entire unit mapping through a uniqueness check.

To be thorough, maybe also add a loop through all prefixes to make sure there are no overlaps with prefixed units either.

Copy link
Author

Choose a reason for hiding this comment

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

Probably due to both "pint" and "point" having "pt" as an alternative designation?

Definitely that.

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.

None yet

2 participants