Skip to content

Conversation

@ipspace
Copy link
Owner

@ipspace ipspace commented Dec 4, 2025

The commit gives 'netlab validate' the ability to reread the validation tests if the topology file has changed, or read them from the specified source file.

Also changed:

  • More control over the warnings generated by the 'read topology from the snapshot' process
  • Store the name of the snapshot and modified topology file in the '_input' element

Implements #2876

The commit gives 'netlab validate' the ability to reread the validation
tests if the topology file has changed, or read them from the specified
source file.

Also changed:
* More control over the warnings generated by the 'read topology from
  the snapshot' process
* Store the name of the snapshot and modified topology file in the
  '_input' element
Copilot finished reviewing on behalf of ipspace December 4, 2025 17:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the netlab validate command to automatically reread validation tests when the lab topology file has been modified since the lab started, eliminating the need to restart the lab during test development. It also adds a --source option to read tests from a separate file.

Key changes:

  • Added automatic detection and reloading of validation tests when topology source files are modified
  • Added --source CLI option to specify an alternative file for validation tests
  • Enhanced snapshot loading with control over modification warnings

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
netsim/cli/validate/source.py New module containing validation test loading and attribute validation logic
netsim/cli/validate/parse.py Added --source CLI argument and improved help text grammar
netsim/cli/validate/init.py Integrated source module and added logic to reload tests when topology files are modified
netsim/cli/init.py Enhanced load_snapshot and check_modified_source to support optional warnings and track modified files
docs/netlab/validate.md Updated usage documentation and added section on validation test development workflow


Validation test development is usually an interactive process that requires several changes to the **validate** lab topology attribute before you get them just right. Restarting the lab every time you change the validation tests just to have them transformed and stored in the snapshot file is tedious; these changes to **netlab validate** (introduced in release 25.12) streamline the process:

* The **netlab validate** command compares the timestamp of the lab topology file with the timestamp of the snapshot file. When necessary, it rereads the validation tests from the changed lab topology file.
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The documentation states that netlab validate will "reread" tests when the topology file changes, but the implementation always uses the first file (topology.input[0]) rather than the modified file (topology._input.modified). This could lead to reading tests from an unchanged defaults file instead of the modified topology file.

Suggested change
* The **netlab validate** command compares the timestamp of the lab topology file with the timestamp of the snapshot file. When necessary, it rereads the validation tests from the changed lab topology file.
* The **netlab validate** command compares the timestamp of the lab topology file with the timestamp of the snapshot file. When necessary, it rereads the validation tests from the first input file (usually the lab topology file), not necessarily from a modified file if multiple inputs are used.

Copilot uses AI. Check for mistakes.

def update_validation_tests(topology: Box, src: str) -> None:
log.info(f'Reading validation tests from {src}')
add_topo = read.read_yaml(filename=src) # Read tests or whole topology from input file
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

If read.read_yaml returns None (e.g., when the file doesn't exist), the error message "The input file is not a dictionary" is misleading. Consider checking for None explicitly and providing a clearer error message like "Cannot read the input file" or "The input file does not exist".

Suggested change
add_topo = read.read_yaml(filename=src) # Read tests or whole topology from input file
add_topo = read.read_yaml(filename=src) # Read tests or whole topology from input file
if add_topo is None:
error_and_exit('Cannot read the input file or the file does not exist')

Copilot uses AI. Check for mistakes.
log.warning(
text=f'Topology source file(s) have changed since the lab has started',
module='-')
source.update_validation_tests(topology,topology.input[0])
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The code reads validation tests from topology.input[0] (the first input file), but topology._input.modified contains the path to the modified file, which might not be the first one in the list. Consider using topology._input.modified directly instead of topology.input[0] to ensure the modified file is used.

Suggested change
source.update_validation_tests(topology,topology.input[0])
source.update_validation_tests(topology,topology._input.modified)

Copilot uses AI. Check for mistakes.
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