-
Notifications
You must be signed in to change notification settings - Fork 437
Test Case LR6 #1539
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
base: development
Are you sure you want to change the base?
Test Case LR6 #1539
Conversation
| mocha.describe('for line charts', () => { | ||
| mocha.describe('for range (min/max)', () => { | ||
| mocha.describe('for quantity meters', () => { | ||
| expectRangeToEqualExpected, |
There was a problem hiding this comment.
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) => { |
There was a problem hiding this comment.
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).
|
Thank you for your contribution to OED. I've verified that you've signed the CLA and have made some suggestions for changes. |
|
@tinisha-davis It has been a week since comments were made on this PR. I wanted to see if you needed anything. |
|
Hey,
Thanks for checking in. The main reason it’s been a week is because I got cut with glass on my left hand, hand stitches put in and taken out and I’m having surgery on it this upcoming Monday.
When you left the comment, I started working on reformatting and using the helper method right away. I did run into an issue with the helper method, the LR6 test has null values and the helper method doesn’t accept null values. That’s what prevented me from being able to resubmit the same day. Paired with my hand issues and it’s taking me longer than I expected to finish the task! I’m hoping I can get it working before Mondays surgery.
In all fairness I haven’t dedicated much time to it due to doctor appointments but I have today and tomorrow off and so I’d like to get it finished this week.
If you have any advice about the helper method and null values I’d appreciate it!
Thank you for being a rad maintainer!
Tinisha Davis
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Steven Huss-Lederman ***@***.***>
Sent: Thursday, September 25, 2025 6:16:22 AM
To: OpenEnergyDashboard/OED ***@***.***>
Cc: Davis, Tinisha ***@***.***>; Mention ***@***.***>
Subject: Re: [OpenEnergyDashboard/OED] Test Case LR6 (PR #1539)
[https://avatars.githubusercontent.com/u/987062?s=20&v=4]huss left a comment (OpenEnergyDashboard/OED#1539)<#1539 (comment)>
@tinisha-davis<https://github.com/tinisha-davis> It has been a week since comments were made on this PR. I wanted to see if you needed anything.
—
Reply to this email directly, view it on GitHub<#1539 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BLDIFHCFGZ4TDQFOQ6HHVIT3UPTKNAVCNFSM6AAAAACGZLSIC6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMZTHE3DKMBUGE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
CAUTION: This email originated outside of the Seattle Colleges’ email system. Do not click links or open attachments unless you recognize the sender and know the content is safe. Questions? Contact IT Services at x6333 (Central), x3630 (North), x5844 (South) or email ***@***.***
|
|
@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. |
|
A video call would be awesome! What’s your availability today or tomorrow? The helper method doesn’t return null values correct but the time period the LR6 testing is using includes null values and so those are being passed into the helper function as parameters and it’s not accepting null value parameters. Which does make sense. That’s why I have the hard coded loop instead of the helper method. I’m sure there’s a way to make it work I just haven’t fiddled with it enough yet. This is my first open source project so I feel nervous about changing anything other than LR6 test code.
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Steven Huss-Lederman ***@***.***>
Sent: Thursday, September 25, 2025 8:31:02 AM
To: OpenEnergyDashboard/OED ***@***.***>
Cc: Davis, Tinisha ***@***.***>; Mention ***@***.***>
Subject: Re: [OpenEnergyDashboard/OED] Test Case LR6 (PR #1539)
[https://avatars.githubusercontent.com/u/987062?s=20&v=4]huss left a comment (OpenEnergyDashboard/OED#1539)<#1539 (comment)>
@tinisha-davis<https://github.com/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.
—
Reply to this email directly, view it on GitHub<#1539 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BLDIFHAJMDN66MNJJD3RXGL3UQDDNAVCNFSM6AAAAACGZLSIC6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMZUG42DANRQGM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
CAUTION: This email originated outside of the Seattle Colleges’ email system. Do not click links or open attachments unless you recognize the sender and know the content is safe. Questions? Contact IT Services at x6333 (Central), x3630 (North), x5844 (South) or email ***@***.***
|
@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. |
|
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. |
Description
Test Case LR6
npm test: all tests passing--grep "LR6"Fixes #1536
Type of change
Test Case
Checklist
Limitations