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

Adjust specs for OH4.1 QuantityType changes #145

Closed
wants to merge 1 commit into from

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Sep 13, 2023

@@ -11,7 +11,8 @@
# quantity type operand
expect(QuantityType.new("50 °F") + QuantityType.new("50 °F")).to eql QuantityType.new("100.0 °F")
expect(QuantityType.new("50 °F") - QuantityType.new("25 °F")).to eql QuantityType.new("25.0 °F")
expect((QuantityType.new("100 °F") / QuantityType.new("2 °F")).to_i).to be 50
# @deprecated OH4.0 result of 50. OH 4.1 will result in 1. see https://github.com/openhab/openhab-core/pull/3792
expect((QuantityType.new("100 °F") / QuantityType.new("2 °F")).to_i).to be(50).or be(1)
Copy link

@mherwege mherwege Sep 13, 2023

Choose a reason for hiding this comment

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

If I try this with the DSL, I get 1.212272835575194402928498711200641256820715734904441123168785784940181676 as a result (which is the division of the absolute Kelvin values of the 2 temperatures). Is it a characteristic of jruby this will round to 1? 1 definitely is not logical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It rounds it to 1 because of the call to_i which converts it to integer.

Choose a reason for hiding this comment

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

Then it is maybe better to select other values as input, as this rounding can create a false impression, and is quite meaningless as a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're not verifying the actual behavior of relative vs absolute units, I'm just going to avoid using temperature units in this particular test. We only need to verify that multiplication / division can be done without having to verify the implementation details.

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
@jimtng jimtng marked this pull request as ready for review September 13, 2023 14:45
@jimtng jimtng requested a review from ccutrer September 13, 2023 14:45
@jimtng
Copy link
Contributor Author

jimtng commented Sep 17, 2023

Superseded by #147

@jimtng jimtng closed this Sep 17, 2023
@jimtng jimtng deleted the fix-quantity-specs branch September 18, 2023 01:46
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.

2 participants