Skip to content

Conversation

@tinisha-davis
Copy link

@tinisha-davis tinisha-davis commented Sep 17, 2025

Description

Test Case LR6

  • Added LR6 test for raw points without min/max
  • Ran full test suite with npm test: all tests passing
  • Verified LR6 specifically with --grep "LR6"

Fixes #1536

Type of change

Test Case

  • Note merging this changes the database configuration.
  • This change requires a documentation update

Checklist

  • I have followed the OED pull request ideas
  • I have removed text in ( ) from the issue request
  • You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

mocha.describe('for line charts', () => {
mocha.describe('for range (min/max)', () => {
mocha.describe('for quantity meters', () => {
expectRangeToEqualExpected,
Copy link
Contributor

Choose a reason for hiding this comment

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

Lines 14-29 were modified unnecessarily. Please revert these change or use an auto formatter

// Extract rows from API response
const rows = res.body[METER_ID] || [];
// Loop through each row if API response and compare to corresponding expected row
rows.forEach((row, i) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using a for loop to iterate through and verifying each value manually, please use the function expectRangeToEqualExpected(Response, Expected).

@Chocopepero
Copy link
Contributor

Thank you for your contribution to OED. I've verified that you've signed the CLA and have made some suggestions for changes.

@huss
Copy link
Member

huss commented Sep 25, 2025

@tinisha-davis It has been a week since comments were made on this PR. I wanted to see if you needed anything.

@tinisha-davis
Copy link
Author

tinisha-davis commented Sep 25, 2025 via email

@huss
Copy link
Member

huss commented Sep 25, 2025

@tinisha-davis I'm sorry to hear about your injury and wish you the best in getting through it. The delay is not an issue - I just wanted to know if we could help.

I'm not sure why the test case is returning null values. Any chance you could commit/push your update even though it does not work? I could then look at it to try to figure out what is going on. If you prefer to work together to figure this out then we can arrange a video call. When you are up to it, let me know what you think.

@tinisha-davis
Copy link
Author

tinisha-davis commented Sep 25, 2025 via email

@huss
Copy link
Member

huss commented Sep 25, 2025

A video call would be awesome! What’s your availability today or tomorrow?

@tinisha-davis Today would be easier as I have a lot of appointments tomorrow. Do you use Discord? If so, why don't you message me about when - the OED website has the link to our Discord server and I'm pinned/easy to find. If not, then we can figure something else out.

@huss
Copy link
Member

huss commented Sep 26, 2025

I want to document that @tinisha-davis found a bug in our test code for range. The testing function OED provided does not correctly work for raw/meter data and that is what this test is doing. When this is the case, the min/max values are all null so OED can detect that no min/max should be shown. (It was done this way to be easy instead of not sending back all the null values.) This is why the test had issues since the test function does not consider this. I very much appreciate the efforts of @tinisha-davis in working so hard to change the test code for this case so it worked and reaching out about the issue as it will fix an issue with OED code. Since @tinisha-davis found the issue, they are going to work to update our testing function to work properly and then use the standard, updated method to test the result.

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.

3 participants