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

Prototype test script for TTINTEL-239 #21

Closed
wants to merge 1 commit into from
Closed

Conversation

spenced
Copy link

@spenced spenced commented Apr 28, 2023

No description provided.

Signed-off-by: David Spence <dspence@redhat.com>
@spenced spenced requested review from jnunyez and crwr45 April 28, 2023 11:46
Copy link
Collaborator

@jnunyez jnunyez left a comment

Choose a reason for hiding this comment

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

Went through the code, got one big question:
What is the value of the FSM for the post-processor? (class FSM). Is it maybe aiming at identify root cause of a problem? I really would like to understand the benefit (I'm sure there is).

r'.*$',
))

Parsed = namedtuple('Parsed', ('timestamp', 'interface', 'terror', 'state'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

terror 😱 🤣


def handle_S0_in_TESTING_INITIALIZING(fsm, arg):
"""Handle event S0 in state /TESTING/INITIALIZING"""
fsm.state = STATE_TESTING_INITIALIZING
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not loving the assignments in these functions that have no lasting effect. On first reading it me wonder if there was a side effect of the first assignment that makes it necessary, though I can't see anything to back that up.

What's the purpose of the assignments that set fsm.state to the value it already has?

Copy link
Author

Choose a reason for hiding this comment

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

The state assignments are output on each "state boundary crossing" for an external transition. An external transition is one where the FSM leaves the current state before entering the target state. The state boundaries crossed are all those up to (but not including) the common ancestor of current and target states. With regard to "assigning fsm.state to the value it already has", that will be due to an external transition back into the same state.

Currently I expect these to be optimized away (by Python itself). I've left them in the generated output as they document the boundary crossings, allowing visual inspection that the intended path is taken.

I agree they're ugly. Getting rid of them is on my TODO list.

@spenced
Copy link
Author

spenced commented May 4, 2023

Replying to Jose's comment... for this prototype, the purpose was twofold: 1) to make it easy for me to identify events so I could draw vertical lines in the plot; 2) to stimulate conversation around separation of concerns, how we combine code to produce higher order tests.

@jnunyez
Copy link
Collaborator

jnunyez commented May 7, 2023

Replying to Jose's comment... for this prototype, the purpose was twofold: 1) to make it easy for me to identify events so I could draw vertical lines in the plot; 2) to stimulate conversation around separation of concerns, how we combine code to produce higher order tests.

@crwr45 @spenced Propose to close this PR since this work will be moved to https://github.com/redhat-partner-solutions/vse-sync-pp

@jnunyez jnunyez closed this May 11, 2023
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.

None yet

3 participants