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

Unit Testing Framework #405

Open
wants to merge 58 commits into
base: master
Choose a base branch
from

Conversation

KnockbackNemo
Copy link
Contributor

Quality Assurance Checklist

To make reviews more efficient, please make sure the software feature meets the following standards and check everything off that meets the quality check. Once everything has been checked, the assigned reviewers will begin the review process. Edit this description to check off the list.

There are exceptions with all guidelines. As long as your decisions are justified, then you are good! Contact the reviewers or the leads about any exceptions.

Requirements

  • Followed Coding Style Guide
  • Presented/discussed in some capacity with others on the Controls team
  • Code Build checks pass
  • No merge conflicts
  • Software feature has documentation for it updated in both ReadTheDocs and the comments
  • Software feature has documentation for it updated
  • Testing
    • Software feature has test associated with it
    • Test provides useful information and uses relevant data to accurately represent Controls
    • Tested on hardware
    • NOTE: If test file already exists, use that one
  • If applicable, have you added the new feature to main.c?
  • Tagged appropriate issue ticket on this PR
  • Did you have fun?

Things to Consider

  • Even if the above are checked, is this the best way of writing my code?
    • It's possible to write code that works, but are there ways to make code more efficient?

KnockbackNemo and others added 22 commits December 30, 2023 16:45
…dition of fff using Contactors; added Unity files.
…ST_LEADER checks if there is a valid test file in the Tests folder but unit tests are kept in a different directory. Unit tests currently don't use the Test_ prefix nor do they check for a valid file, but we may want to add these features later on.
…g for ifdefs to see if it helps with including files.
… makefile, made a new Test_Pedals.c in UnitTests as a temporary file to make it compile since the makefile can't seem to find the test in the Tests folder.
…ariable, which didn't take into account the Tests/ directory. Also renamed the output file executable to be .out instead of .o since it is full, linked executable and not just an object file. FInally, add as a prerequisite to compile the unit test. I believe this will tell the Makefile to recompile the executable if any of the C_SOURCES have been changed since it was last compiled.
… public on the master branch, so I'm pretty sure it's okay.
…_INCLUDES now uses the explicit paths for the mocks instead of the wildcard function since it wanted a directory instead of files and also because it needed the -I part appended to each path anyways; New flag -DTEST_ uses the name of the file we're testing and defines it as TEST_<FILE>. This can be used in #ifdefs to determine if we want to include a regular header or a mock header. I'm not sure if putting an ifdef in each mock header is the best way to do it, but it was the easiest way I could think of at the moment. The mock Contactors.h is an example of this. Test changes: Made additional mocks so that Test_Contactors runs; moved the DEFINE_FFF_GLOBALS to the test file since this can only be defined once, and we at least know we'll only be running one test file at a time. In mock headers where we sometimes want to mock and sometimes want to test (ex: Contactors, Apps), the #include_next macros will include the second instance of the file on the search path. This means if we include the mocks before the normal headers in C_INCLUDES, we can use #include_next to choose the normal header. Not the cleanest way of doing things, but hopefully it at least works for now.
…d for OS, contactors working with OS, initial version of RCC
… tasks as the task mocks are not being referenced
…gcc searches the current directory for #include files before the -I directories, so using the real ReadCarCAN.h caused gcc to use the real Tasks.h since it resides in the same directory. Adding -iquote stops the compile from searching the current directory unless it is listed as a -I flag directory, so now fake Tasks.h will still be found first, even if we use a file in the real Apps/Inc!
…nged Makefile a bit more to use deprecated -I- for now since it seems to work better than iquote, which would cause the directory after to become unable to be found.
…hould cover all the parts of the os we need, I think?
@KnockbackNemo KnockbackNemo linked an issue Jan 18, 2024 that may be closed by this pull request
@IshDeshpa
Copy link
Contributor

Might need to merge in master to get stuff to build properly on GH Actions. May have some merge conflicts

… confused / Added a filter to the unit test makefile to ensure we find the correct real source file to test and not accidentally a mock one.
…olar/Controls into feature/unit-testing-framework-394
…es so that things work with the new cleaned-up codebase.
…ests, fixed other small changes related to merge issues so that the unit tests run.
…t this at least tests the logic we probably want to test for a unit test. Also moved some makefile define flags into C_DEFS since only those flags will be compiled with clang format/tidy (we were failing those checks before.
@diyarajon diyarajon marked this pull request as ready for review January 31, 2024 21:34
Copy link
Contributor

@IshDeshpa IshDeshpa left a comment

Choose a reason for hiding this comment

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

Good stuff! A few notes though:

  • Many of the headers for the RTOS and CPU seem like they just do the same thing as the original headers without any difference. If this is the case, why make a duplicate?
  • See LOOP macro comments.
  • See anything else I've put down

.vscode/LHR.code-workspace Outdated Show resolved Hide resolved
Apps/Src/CommandLine.c Outdated Show resolved Hide resolved
Apps/Src/ReadCarCan.c Outdated Show resolved Hide resolved
Apps/Src/Tasks.c Outdated Show resolved Hide resolved
BSP/STM32F413/Makefile Outdated Show resolved Hide resolved
Tests/UnitTests/Tests/Test_Contactors.c Show resolved Hide resolved
Tests/UnitTests/Tests/Test_Contactors.c Show resolved Hide resolved
Tests/UnitTests/Tests/Test_ReadCarCan.c Outdated Show resolved Hide resolved
Tests/UnitTests/Tests/Test_Pedals.c Outdated Show resolved Hide resolved
Tests/UnitTests/Mocks/RTOS/os.h Outdated Show resolved Hide resolved
@diyarajon diyarajon requested a review from IshDeshpa March 2, 2024 22:46
@KnockbackNemo KnockbackNemo linked an issue Mar 2, 2024 that may be closed by this pull request
@IshDeshpa
Copy link
Contributor

Merge in master pls

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.

Unit Testing Framework Specific test macro for each test file
4 participants