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.

Sign up for GitHub

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 #1780, RTEMS CFE_FT_Global build failure #1781

Merged
merged 1 commit into from
Aug 10, 2021
Merged

Fix #1780, RTEMS CFE_FT_Global build failure #1781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/cfe_testcase/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ include_directories(inc)

# Filenames based on doxygen groups.
# Create the app module
add_cfe_app(cfe_testcase
src/cfe_test_table.c
add_cfe_app(cfe_testcase
src/cfe_test.c
src/cfe_test_table.c
src/es_application_control_test.c
src/es_info_test.c
src/es_task_test.c
Expand Down
5 changes: 5 additions & 0 deletions modules/cfe_testcase/src/cfe_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ CFE_FT_Global_t CFE_FT_Global;
*/
void CFE_TestMain(void)
{
/* Constant Table information used by all table tests */
CFE_FT_Global.TblName = "TestTable";
CFE_FT_Global.RegisteredTblName = "CFE_TEST_APP.TestTable";
CFE_FT_Global.TblFilename = "test_tbl.tbl";
Comment on lines +44 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is just used by cfe_test_table.c it's probably more appropriate to initialize there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used by multiple functions across multiple source files. The alternative is to re-initialize in each one of them or initialize them in the first one but that introduces a test execution order dependency which feels too brittle.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case I'm good as-is.

Copy link
Contributor Author

@astrogeco astrogeco Aug 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Specifically

TBLContentAccessTestSetup();
TBLContentMangTestSetup();
TBLInformationTestSetup();
TBLRegistrationTestSetup();

An alternative would be to create a new function say TBLTests_Setup that only contains this initialization.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually prefer the idea of making a "Setup" function here that does this init, then it can be passed as the setup function as part of the UtTest_Add for any test that uses this table struct. That being said, its OK as is, I wouldn't hold this up.

Copy link
Contributor

@nmullane nmullane Aug 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think doing it as part of setup function would work out because there is already a setup function which is used by most but not all of the table tests to register a table. Also since CFE_FT_Global is already instantiated in cfe_test.c I think it would make the most sense to also perform the initialization of this data just the once either there or perhaps it could be moved into cfe_test_table.c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened an issue for future work #1797


/*
* Register this test app with CFE assert
*
Expand Down
5 changes: 1 addition & 4 deletions modules/cfe_testcase/src/cfe_test_table.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,10 @@
#include "cfe_test.h"
#include "cfe_test_table.h"

/* Constant Table information used by all table tests */
CFE_FT_Global_t CFE_FT_Global = {
.TblName = "TestTable", .RegisteredTblName = "CFE_TEST_APP.TestTable", .TblFilename = "test_tbl.tbl"};

/* Setup function to register a table */
void RegisterTestTable(void)
{

UtAssert_INT32_EQ(CFE_TBL_Register(&CFE_FT_Global.TblHandle, CFE_FT_Global.TblName, sizeof(TBL_TEST_Table_t),
CFE_TBL_OPT_DEFAULT, NULL),
CFE_SUCCESS);
Expand Down
2 changes: 0 additions & 2 deletions modules/cfe_testcase/src/cfe_test_table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
*/
#include "cfe_test.h"

CFE_FT_Global_t CFE_FT_Global;

void RegisterTestTable(void);
void UnregisterTestTable(void);

Expand Down