Skip to content

Conversation

@m-aleem
Copy link
Collaborator

@m-aleem m-aleem commented Jun 26, 2025

In support of nasa-fprime#3446 Similar to components, add a default <X>Tester to state machine classes, in order to support white-box testing

@m-aleem m-aleem requested a review from bocchino June 26, 2025 23:44
Copy link
Collaborator

@bocchino bocchino 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 good start, but I think a bit more design is required. It looks like this approach uses the unqualified name of the state machine implementation class to create a friend for the state machine base class, and there's no friend for the derived implementation class. Instead, I suggest that we do the following:

  1. Use two friends, one for the base class and one for the derived implementation class. The base class friend should be inserted here, and the derived class friend should be inserted in the code that instantiates a state machine in a component. See here for an example:
    class FppTest_SmInitial_Basic :
    public FppTest::SmInitial::BasicStateMachineBase
    .
  2. Use the fully-qualified names for the friends, as the code generator does for the class names in the example above. In the example above, the names of the friends could be FppTest::SmInitial::BasicStateMachineBaseTester for the base class and FppTest_SmInitial_BasicTester for the derived class. It's a little more verbose, but it is more structured and less likely to create name collisions. Also, it's consistent with the existing code gen strategy.

@bocchino
Copy link
Collaborator

Another approach would be to use the same friend name for both the base class and derived class. Maybe that would be simpler on the testing side, since one tester class could access both the base class and the derived class in the implementation under test. If we do that, then we can use the fully-qualified name of the derived class (e.g., FppTest_SmInitial_BasicTester in the example above) for the friend in both places.

@bocchino
Copy link
Collaborator

Yet another option would be to make the component tester class a friend of the state machine implementation class inside the component. Maybe that is the best for testing the component. So in the example above we could use (1) FppTest::SmInitial::BasicStateMachineBaseTester for the friend of the state machine base class and (2) SmInitialActiveTesterBase and SmInitialActiveTester as the friends of the derived class inside the component. That would allow the component tester classes to peer inside the derived class, so we could convert the PRIVATEcode gen there to private.

@bocchino
Copy link
Collaborator

So in the example above we could use (1) FppTest::SmInitial::BasicStateMachineBaseTester for the friend of the state machine base class

Or maybe it's better to use FppTest::SmInitial::BasicTester as the friend class name (similarly to what is currently done, but with qualification), so that the state machine tester can access the base class. I suspect that may play better with what is done in FppTest, and that's why you used the derived class name as the base class friend.

@bocchino
Copy link
Collaborator

So after all that, my suggestion is to do the following:

  1. Keep the derived-class name as the friend in the state machine base class, but add qualification.
  2. Add the component tester and tester base as friends of the state machine implementation in the component generated code.

@bocchino
Copy link
Collaborator

It seems that when you declare a friend F inside a namespace N, the name F is automatically qualified by N. For example, this code doesn't compile if I comment out the second namespace N:

namespace N {

class C {
    friend class F;
  private:
    int x = 0;
};

}

namespace N {

class F {
  public:
    int getX(const N::C& c) { return c.x; }
};

}

So I think what I am suggesting reduces to the second item above, i.e., add the component tester and tester base as friends of the state machine implementation in the component generated code.

@m-aleem m-aleem requested a review from bocchino June 29, 2025 05:22
@m-aleem
Copy link
Collaborator Author

m-aleem commented Jun 29, 2025

I think I have addressed the requested updates, so using the example above now in file SmInitialActiveComponentAc.ref.hpp classes derived from FppTest::SmInitial::BasicStateMachineBase will have as friends SmInitialActiveTesterBase & SmInitialActiveTester (the same/existing friends as of SmInitialActiveComponentBase)

Let me know if this addresses the requested changes, happy to iterate further if needed.

Revise comments
Update test harness to comply with changes to F Prime
Copy link
Collaborator

@bocchino bocchino left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I revised the comments in the generated code, and I updated the test harness for fpp-check.

@bocchino bocchino merged commit 7489ecd into nasa:main Jul 7, 2025
14 checks passed
@m-aleem m-aleem deleted the aleem branch July 28, 2025 20:07
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