Skip to content

ctests: stop including C files in the test files#432

Merged
daniloegea merged 1 commit intocanonical:mainfrom
daniloegea:ctests_improvements
Jan 16, 2024
Merged

ctests: stop including C files in the test files#432
daniloegea merged 1 commit intocanonical:mainfrom
daniloegea:ctests_improvements

Conversation

@daniloegea
Copy link
Contributor

@daniloegea daniloegea commented Jan 5, 2024

With these changes, a copy of libnetplan without static functions will be build. All the test files will be linked against this separate library. With that, there is no need to include the C files in the test files anymore.

To make this possible, all the static keywords in function declarations were replaced with a macro that will be defined at compilation time.

Description

My goal is to make #425 easier to be implemented. Also, refactoring the C code is less likely to break the build of test files...

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@daniloegea daniloegea requested a review from slyon January 5, 2024 17:56
@slyon
Copy link
Contributor

slyon commented Jan 8, 2024

This approach sounds reasonable to me. Especially with the default STATIC -> static fallback in place, as long as UNITTESTS is not defined.

CC @fnordahl WDYT? Would that work for you?

With these changes, a copy of libnetplan without static functions will be
build. All the test files will be linked against this separate
library. With that, there is no need to include the C files in the test
files anymore.

To make this possible, all the static keywords in function declarations
were replaced with a macro that will be defined at compilation time.
@daniloegea
Copy link
Contributor Author

Rebased on top of the last batch of commits.

@daniloegea daniloegea marked this pull request as ready for review January 16, 2024 16:42
Copy link
Contributor

@slyon slyon left a comment

Choose a reason for hiding this comment

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

This is a nice refactoring that makes testing easier. It doesn't affect the production code.

LGTM, let's get this merged!

@daniloegea daniloegea merged commit 517d0f8 into canonical:main Jan 16, 2024
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.

2 participants