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

Failing UT fixes #185

Merged
5 commits merged into from Nov 12, 2018
Merged

Failing UT fixes #185

5 commits merged into from Nov 12, 2018

Conversation

ghost
Copy link

@ghost ghost commented Nov 1, 2018

Description

Two unit tests fail after applying the -fshort-enums flag.

To fix the LTC4275 update_status test, either need to change the functionality of update_status to have an else after this if or need to change expected return values that there is no else and so values should be unmodified. Went with the second option for now, so expecting values to be unchanged and compare with the values of state and pdAlert from before the update_status is called.
While I was editing this test suite, I also updated all integer expect values to be the correct enum value instead.

To fix the ocmp_ltc4275 test fail, I just increased the maximum number of GPIO pin callbacks in the GPIO stub to be 32, which is greater than the index of 27 used in the failing test. This isn't really a fix for the underlying issue, so I also added a check to the index so that future tests should fail if they index beyond FAKE_GPIO_PIN_COUNT.

Test Plan

Verify Fixes known issues
Run Unit Tests (make clean test) and verify that all tests now pass.

Verify Index Check fails correctly
Modify FAKE_GPIO_PIN_COUNT to be small (set to 1) and verify that this causes tests to fail because of returning GPIO error and not because of writing incorrect memory space. (test_ina226_alerts, test_se98a_alerts, and test_LTC4015_alerts all fail).
If running the same thing without the index check in fake_GPIO.c, the tests do not finish because of memory corruption.

Issues

Fixes #63
Fixes #113

@ghost ghost added bug FW UnitTest Firmware Unit test cases labels Nov 1, 2018
@ghost ghost added this to In Progress in Release 0.2.1 via automation Nov 1, 2018
@ghost ghost mentioned this pull request Nov 1, 2018
Copy link
Collaborator

@JoshuaJeyaraj JoshuaJeyaraj left a comment

Choose a reason for hiding this comment

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

To fix the ocmp_ltc4275 test fail, I just increased the maximum number of GPIO pin callbacks in the GPIO stub to be 32, which is greater than the index of 27 used in the failing test. This isn't really a fix for the underlying issue, so I also added a check to the index so that future tests should fail if they index beyond FAKE_GPIO_PIN_COUNT.

Nice catch !!
I am not sure how this issue is not showing up in the cygwin environment.
I think the max number should be increased to OC_EC_GPIOCOUNT , the rest looks good to me

@jspectacular
Copy link
Collaborator

jspectacular commented Nov 8, 2018

coverage target does not work. e.g. make coverage fails. Not to be confused with make cov, which does work. The coverage target only works when invoked by some other rule. Otherwise, the unit test summary shows 0 total failures.

@ghost ghost merged commit 3bedc18 into master Nov 12, 2018
Release 0.2.1 automation moved this from In Progress to Done Nov 12, 2018
@ghost ghost deleted the failing_ut_fix branch November 12, 2018 16:59
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug FW UnitTest Firmware Unit test cases
Projects
No open projects
Release 0.2.1
  
Done
Development

Successfully merging this pull request may close these issues.

Bug: Unit Tests FakeGpio_setCallback can index past array length Bug: Test LTC4275 Update_status() Fails
2 participants