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

Fix #80, Unit tests read past array bounds and general cleanup #98

Merged
merged 5 commits into from
Aug 31, 2023

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Jul 26, 2023

Checklist (Please check before submitting)

Describe the contribution

Also consolidated "result" asserts using modern APIs, removed low-value/high-maintenance checks on event string and type (ID is sufficient to confirm path).

Recommend a follow-on issue to replace the rest of the UtAssert_True uses.

Testing performed
CI (including coverage)

Expected behavior changes
Greatly reduced technical debt wrt unit tests (large reduction in lines, and consistent table initialization)

System(s) tested on
CI

Additional context
Note this should make the remaining bug fixes and updates easier, by reducing ut change overhead.

Could squash if CCB wants, just tried to break down the changes a little.

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the Equuleus milestone Jul 26, 2023
@dzbaker dzbaker merged commit 2733b00 into nasa:main Aug 31, 2023
16 checks passed
@jphickey
Copy link
Contributor

In retrospect I don't know why we didn't merge this into equuleus-rc1. I guess I didn't realize at the time we reviewed how horrendously broken the tests were. At the same time we were trying to limit new stuff in the build and this looked like a big change. Although they somehow ran most of the time, I see now that the tests were quite bad, we really should have merged this right away.

We need to somehow encourage stakeholders to update their SC now....

@skliper skliper deleted the fix80-ut_read_past_bounds branch April 1, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SC_LoadAts_Test_AtsEntryOverflow Unit Test indexes past array bounds
3 participants