Skip to content

Fix # 35: Created SB checkout test#54

Open
czogby-nasa wants to merge 3 commits into
devfrom
35-create-sb-checkout-test
Open

Fix # 35: Created SB checkout test#54
czogby-nasa wants to merge 3 commits into
devfrom
35-create-sb-checkout-test

Conversation

@czogby-nasa
Copy link
Copy Markdown
Contributor

No description provided.

@czogby-nasa czogby-nasa requested a review from ddstewar May 11, 2026 18:36
@czogby-nasa czogby-nasa self-assigned this May 11, 2026
@czogby-nasa czogby-nasa linked an issue May 11, 2026 that may be closed by this pull request
1 task
@czogby-nasa czogby-nasa added this to the v7.1.0 milestone May 11, 2026
Copy link
Copy Markdown
Contributor

@ddstewar ddstewar left a comment

Choose a reason for hiding this comment

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

Fixes needed.

Comment thread targets/CFS/procedures/cfs_test_groups_for_cfe/cfe_sb_checkout.py Outdated
Comment thread targets/CFS/procedures/cfs_test_groups_for_cfe/cfe_sb_checkout.py Outdated
Comment thread targets/CFS/procedures/cfs_test_groups_for_cfe/cfe_sb_checkout.py Outdated
Comment thread targets/CFS/procedures/cfs_test_groups_for_cfe/cfe_sb_checkout.py Outdated
Comment thread targets/CFS/procedures/cfs_test_groups_for_cfe/cfe_sb_checkout.py Outdated
Comment thread targets/CFS/procedures/cfs_test_groups_for_cfe/cfe_sb_checkout.py Outdated
@czogby-nasa
Copy link
Copy Markdown
Contributor Author

czogby-nasa commented May 11, 2026

Fixes needed.

I tried running without the FM commands. CFE_SB_CMD_WRITE_PIPE_INFO gives the following error:
CFE_SB 40: Error creating file /cf/func_file_pipe.dat, stat=0xc8000006

So clearly the FM command before that is required. And it seems safer to also include the other 2, since they're similar commands. Only costs 0.15 of a second each.

@ddstewar
Copy link
Copy Markdown
Contributor

ddstewar commented May 12, 2026

Fixes needed.

I tried running without the FM commands. CFE_SB_CMD_WRITE_PIPE_INFO gives the following error: CFE_SB 40: Error creating file /cf/func_file_pipe.dat, stat=0xc8000006

So clearly the FM command before that is required. And it seems safer to also include the other 2, since they're similar commands. Only costs 0.15 of a second each.

All of the SB commands that create files take a filename as a parameter. Change the name to something that does not already exist. It doesn't have anything to do with the execution time of (0.15 as you say) of the commands. We cannot have a cFE app dependent on a cFS optional app for testing. Besides that, the get_info commands do absolutely nothing in this test. The get info commands are there just to verify the file created exists, and there is no logic here verifying that command was accepted at all so they are useless. Regardless, that is an FM command, which is an optional cFS application and not a part of the cFE, so it and all of the FM commands need removed. This test needs to stand alone.

I just tested the CFE_SB_CMD_WRITE_ROUTING_INFO command with the filename /cf/test_rt_info.dat and it worked fine and was accepted, multiple times with the same filename.

I then tried the CFE_SB_CMD_WRITE_MAP_INFO command with the filename /cf/test_map_info.dat and it worked fine and was accepted, multiple times with the same filename.

I finally tested the CFE_SB_CMD_WRITE_PIPE_INFO command with the filename /cf/test_pipe_info.dat and it also worked fine and was accepted, multiple times with the same filename.

Terminal output:

EVS Port1 1980-012-14:15:28.48556 66/1/CFE_SB 39: /cf/test_rt_info.dat written:Size=5836,Entries=91
.EVS Port1 1980-012-14:15:32.48789 66/1/CFE_SB 39: /cf/test_map_info.dat written:Size=792,Entries=91
EVS Port1 1980-012-14:15:35.49044 66/1/CFE_SB 39: /cf/test_pipe_info.dat written:Size=1624,Entries=64

I tried the pipe info command with /cf/func_file_pipe.dat as well, since you said that did not work, and that was accepted too, multiple times in a row with no errors. I do not know what is causing your command to not work. Perhaps you are not on the up to date cFS?

EVS Port1 1980-012-14:17:46.97860 66/1/CFE_SB 39: /cf/func_file_pipe.dat written:Size=1624,Entries=64

@czogby-nasa
Copy link
Copy Markdown
Contributor Author

czogby-nasa commented May 12, 2026

Change the name to something that does not already exist.

The files already existing wasn't an issue for 2 of the commands. I had already checked the /cf/ directory and saw that all 3 files existed for the 3 similar commands, but were only causing an error for one.

It doesn't have anything to do with the execution time of (0.15 as you say)

No, I never said that it had anything to do with that. Read more carefully. I meant that the FM commands only take a negligible amount of time and thus aren't worth worrying about how much time they take.

We cannot have a cFE app dependent on a cFS optional app for testing.

Tell that to the people who wrote the SB functional test this was simplified from (written by Mike Yang, with later changes by Keegan). Do we need to open a ticket to reformulate Mike and Keegan's SB functional test to not use any CFS apps?

There have been discussions in the past about whether or not it's okay to use other apps in tests for an app. I recall that the conclusions were that if there's no other straightforward way then that's what needs to be done. I don't remember if those discussions considered the separation of CFE vs CFS, but they may have, and the crossover exists in at least one existing test (SB), and probably other tests.

The get info commands are there just to verify the file created exists

Yes, these aren't needed and clearly should be removed.

I do not know what is causing your command to not work. Perhaps you are not on the up to date cFS?

I did need to update and rebuild cFS yesterday, because the TBL checkout test I was developing was getting the "no working buffers" error that had been recently discussed on Teams. That fixed the "no working buffers" error. I'm in the process now of seeing if I can get the SB checkout test working without the FM commands, after updating cFS.

@czogby-nasa
Copy link
Copy Markdown
Contributor Author

Change the name to something that does not already exist.

Files written into the /cf/ directory stick around forever until manually deleted, even between resets. For checkout tests, is it okay to assume that a custom filename hasn't already been created? Will the workflow these will run under reset the /cf/ directory when called for the checkout test suite? I could change it to write into /ram/ instead, but that would still require the FSW to have been reset since the last time this test was run - is that a reasonable thing to assume has already happened?

@ddstewar
Copy link
Copy Markdown
Contributor

Change the name to something that does not already exist.

The files already existing wasn't an issue for 2 of the commands. I had already checked the /cf/ directory and saw that all 3 files existed for the 3 similar commands, but were only causing an error for one.

I ran all of the command multiple times with the same file names for both. I saw no error for the command you specified, even when using the same filename. that's why I asked what version you were using. a make distclean would remove all files so if there were locks on a file that should have been removed as well.

It doesn't have anything to do with the execution time of (0.15 as you say)

No, I never said that it had anything to do with that. Read more carefully. I meant that the FM commands only take a negligible amount of time and thus aren't worth worrying about how much time they take.

You said it only takes .15 seconds, implying that the time doesn't matter. I'm saying the time is not the issue with these comments. The test can NOT rely on apps outside of the cFE. that is the issue with having file manager commands in an SB CHECKOUT test.

We cannot have a cFE app dependent on a cFS optional app for testing.

Tell that to the people who wrote the SB functional test this was simplified from (written by Mike Yang, with later changes by Keegan). Do we need to open a ticket to reformulate Mike and Keegan's SB functional test to not use any CFS apps?

You were assigned the task of creating a checkout test for the app. The previous test should have nothing to do with this other than a baseline of what commands are available and how to format them. It was up to you to determine the proper parameters for the checkout test. Which is supposed to be an isolated test of a single app's command acceptance that runs fast. I have provided the comments to tell you that what has been done is not correct and needs changed. The other test will be refactored later. I don't know why it would matter what someone else wrote in a completely different test with completely different goals and trajectory.

There have been discussions in the past about whether or not it's okay to use other apps in tests for an app. I recall that the conclusions were that if there's no other straightforward way then that's what needs to be done. I don't remember if those discussions considered the separation of CFE vs CFS, but they may have, and the crossover exists in at least one existing test (SB), and probably other tests.

There may be exceptions in other tests, but these particular tests need to rely only on themselves or cFE. Going forward if a future test or refactor needs to send commands to other cFS apps that will need protected from trying to execute that portion in the event that app is not present. If commanding changes to another app the Sample_app should be used whenever possible. We have to limit the dependance on cFS apps to an absolute minimum because theese tests can and will be run in systems that do not contain the app it is dependent on. All that is required to run 1 cFS is the cFE, therefor we can only rely on the cFE and the app that is being tested. the Sample_app is the first go-to as it is a model for howto build a cFS app and is easy to integrate.

I do not know what is causing your command to not work. Perhaps you are not on the up to date cFS?

I did need to update and rebuild cFS yesterday, because the TBL checkout test I was developing was getting the "no working buffers" error that had been recently discussed on Teams. That fixed the "no working buffers" error. I'm in the process now of seeing if I can get the SB checkout test working without the FM commands, after updating cFS.

Make sure to pull the current dev branch from https://github.com/nasa/cFS.
make distclean to clean all prior runs.

@ddstewar
Copy link
Copy Markdown
Contributor

deleted, even between resets. For checkout tests, is it okay to assume that a custom filename hasn't already been created?

The checkout tests will be run primarily in a git runner. so the system will be clean every run. That's why I don't care about any cleanup around these tests.

the file existing already did not matter for me when I ran all 3 commands. I was able to send them each multiple times back to back and they were accepted every time.

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.

Create SB checkout test

2 participants