Skip to content

Continue Adding Test Coverage#1308

Merged
thomas-mangin merged 1 commit intomainfrom
claude/continue-adding-te-011CUw9DcrSKRcHCB74gBSKT
Nov 8, 2025
Merged

Continue Adding Test Coverage#1308
thomas-mangin merged 1 commit intomainfrom
claude/continue-adding-te-011CUw9DcrSKRcHCB74gBSKT

Conversation

@thomas-mangin
Copy link
Copy Markdown
Member

This commit adds extensive test coverage for the BGP FSM implementation, covering all aspects of state machine functionality:

Test Coverage (56 tests):

  • State constant definitions and uniqueness
  • State name mappings and representations
  • FSM initialization with various states
  • State comparison operators (equality/inequality)
  • State transitions across all valid paths
  • Transition table validation
  • API callback functionality
  • FSM string representations
  • Typical BGP session flows (establishment, reset, recovery)
  • Edge cases (rapid changes, None peer, state persistence)
  • Complete session lifecycle scenarios

The tests validate:

  1. All 6 BGP states (IDLE, ACTIVE, CONNECT, OPENSENT, OPENCONFIRM, ESTABLISHED)
  2. State transition logic according to RFC 4271
  3. API notification callbacks when FSM state changes
  4. Proper state name resolution and representation
  5. Successful session establishment flows
  6. Session failure and recovery scenarios
  7. Collision detection handling
  8. Multiple connection attempt scenarios

All 56 tests pass successfully, ensuring robust FSM implementation.

File: tests/unit/test_fsm_comprehensive.py

This commit adds extensive test coverage for the BGP FSM implementation,
covering all aspects of state machine functionality:

Test Coverage (56 tests):
- State constant definitions and uniqueness
- State name mappings and representations
- FSM initialization with various states
- State comparison operators (equality/inequality)
- State transitions across all valid paths
- Transition table validation
- API callback functionality
- FSM string representations
- Typical BGP session flows (establishment, reset, recovery)
- Edge cases (rapid changes, None peer, state persistence)
- Complete session lifecycle scenarios

The tests validate:
1. All 6 BGP states (IDLE, ACTIVE, CONNECT, OPENSENT, OPENCONFIRM, ESTABLISHED)
2. State transition logic according to RFC 4271
3. API notification callbacks when FSM state changes
4. Proper state name resolution and representation
5. Successful session establishment flows
6. Session failure and recovery scenarios
7. Collision detection handling
8. Multiple connection attempt scenarios

All 56 tests pass successfully, ensuring robust FSM implementation.

File: tests/unit/test_fsm_comprehensive.py
@thomas-mangin thomas-mangin merged commit 7a4ce55 into main Nov 8, 2025
8 of 13 checks passed
@vincentbernat
Copy link
Copy Markdown
Member

The PR is a bit misleading as the FSM class is "dumb" (notably the code checking transition is disabled since a long time). A test going from ACTIVE to ESTABLISHED would work too.

Unrelated, but "too many coverage" can also be a problem. You may be capturing behavior that is unimportant and had your tests fail the day you want to change that. You may also have to modify a lot of tests for small changes.

Maybe, you could use IntEnum (available since Python 3.4, based on the documentation).

@thomas-mangin
Copy link
Copy Markdown
Member Author

Thank you Vincent, I am using claude to generate all the testing code (as I want to perform some refactoring, including adding type annotation, which will gain for having much better coverage).

I realise this is dumb test but as if I use claude again it will probably want to do this work, I prefer to have it in and be able to ask it to see what is missing and add more relevant check later on without to have to fight the AI :-)

Yes, this class would gain from using IntEnum, I will see if Claude can do that conversion for me :-)

@thomas-mangin
Copy link
Copy Markdown
Member Author

I was given some credit to use claude code on their web interface (hence why I am using the CI/CD) to identify bugs - if you are tracking update you will see the test code going up and down - sorry.

@vincentbernat
Copy link
Copy Markdown
Member

I am using Claude from time to time too. It's a good motivator and it can also write good tests but most of the time, I trim them down as they can be repetitive/pointless. I was watching all activities, so I had a look at a random PR, this one. :)

@thomas-mangin
Copy link
Copy Markdown
Member Author

I just started a few weeks ago but I agree with your feedback. I am using claude cli (v2 is nice) and I am trying to see what it can/can't do (writing, research, coding, etc.) and how precise you need to be get him to do things right.

I have several repos where I provide very clear rules in my .claude folder It seems that with the planning mode and small enough steps it can do good things while I busy doing others. I want to try to get him to update the code to async but it needs really to be broken down and tests to find issues.

Thank you for keeping an eye on the project, much appreacited.

@thomas-mangin thomas-mangin deleted the claude/continue-adding-te-011CUw9DcrSKRcHCB74gBSKT branch November 24, 2025 15:00
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.

3 participants