Add test for feat_req__persistency__multiple_kvs

[igorostrowskiq]

No description provided.

[Copilot]

Pull request overview

This PR adds a new integration test for verifying that multiple KVS (Key-Value Store) instances with different IDs can store and retrieve independent values without interference. The implementation includes both Rust test scenarios and Python test cases, along with necessary helper modules for KVS instance management and parameter handling.

Key changes:

• New test scenario multiple_instance_ids to verify multiple KVS instances operate independently
• Helper modules for KVS parameter parsing and instance creation
• Refactored code organization by moving runtime helpers into a kyron subdirectory
• Extracted common test property decorator into a shared module

Reviewed changes

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

Show a summary per file

File	Description
feature_integration_tests/rust_test_scenarios/src/scenarios/persistency/multiple_kvs_per_app.rs	Implements the Rust test scenario that creates two KVS instances, stores different values, and verifies they persist independently
feature_integration_tests/rust_test_scenarios/src/scenarios/persistency/mod.rs	Defines the persistency scenario group containing the multiple instance IDs test
feature_integration_tests/rust_test_scenarios/src/scenarios/mod.rs	Registers the new persistency scenario group with the root scenario group
feature_integration_tests/rust_test_scenarios/src/scenarios/basic/orchestration_with_persistency.rs	Updates import path to reflect reorganized runtime helper location
feature_integration_tests/rust_test_scenarios/src/scenarios/basic/mod.rs	Simplifies code by importing OrchestrationWithPersistency directly
feature_integration_tests/rust_test_scenarios/src/internals/persistency/mod.rs	Exports KVS helper modules for test scenarios
feature_integration_tests/rust_test_scenarios/src/internals/persistency/kvs_parameters.rs	Provides serde-compatible structure for deserializing KVS configuration parameters
feature_integration_tests/rust_test_scenarios/src/internals/persistency/kvs_instance.rs	Helper function to create KVS instances from test parameters
feature_integration_tests/rust_test_scenarios/src/internals/mod.rs	Reorganizes internals by adding kyron and persistency modules
feature_integration_tests/rust_test_scenarios/src/internals/kyron/mod.rs	New module containing Kyron-related helpers including runtime helper
feature_integration_tests/python_test_cases/tests/persistency/multiple_kvs_per_app.py	Python test case that verifies the multiple KVS instances scenario
feature_integration_tests/python_test_cases/tests/basic/test_orchestartion_with_persistency.py	Refactors to use shared test properties decorator
feature_integration_tests/python_test_cases/test_properties.py	Extracts test properties decorator into shared module for reuse
feature_integration_tests/python_test_cases/fit_scenario.py	Adds ResultCode constants for better readability of exit codes
feature_integration_tests/python_test_cases/BUILD	Updates build configuration to include new test_properties module

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The scenario name multiple_instance_ids doesn't clearly convey that this test verifies multiple KVS instances per application. Consider renaming to multiple_kvs_per_app to match the module name and better describe what's being tested.

Suggested change

	pub struct MultipleInstanceIds;
	impl Scenario for MultipleInstanceIds {
	fn name(&self) -> &str {
	"multiple_instance_ids"
	pub struct MultipleKvsPerApp;
	impl Scenario for MultipleKvsPerApp {
	fn name(&self) -> &str {
	"multiple_kvs_per_app"

[PiotrKorkus]

done

[Copilot]

The struct name MultipleInstanceIds is inconsistent with its scenario name multiple_instance_ids and doesn't clearly indicate it's testing multiple KVS instances per application. Consider renaming to MultipleKvsPerApp to match the module name and improve clarity.

Suggested change

	pub struct MultipleInstanceIds;
	impl Scenario for MultipleInstanceIds {
	pub struct MultipleKvsPerApp;
	impl Scenario for MultipleKvsPerApp {

[PiotrKorkus]

done

[Copilot]

The from_value method expects a nested kvs_parameters field within the provided value, which creates an inconsistent API. The method already receives a value parameter, so requiring another kvs_parameters key inside it is confusing. Consider either removing the nested access or renaming the method to clarify this expectation.

Suggested change

	/// Parse `KvsParameters` from `Value`.
	/// `Value` is expected to contain `kvs_parameters` field.
	pub fn from_value(value: &serde_json::Value) -> Result<Self, serde_json::Error> {
	/// Parse `KvsParameters` directly from a JSON `Value`.
	///
	/// The provided `value` is expected to contain the fields of `KvsParameters`
	/// at its top level (i.e., it should serialize exactly as `KvsParameters`).
	pub fn from_value(value: &serde_json::Value) -> Result<Self, serde_json::Error> {
	serde_json::from_value(value.clone())
	}
	/// Parse `KvsParameters` from a JSON `Value` that wraps it in a
	/// `kvs_parameters` field.
	///
	/// This is useful when the configuration JSON has a structure like:
	///
	/// `{ "kvs_parameters": { ...fields of KvsParameters... } }`
	pub fn from_wrapped_value(value: &serde_json::Value) -> Result<Self, serde_json::Error> {

[github-actions]

The created documentation from the pull request is available at: docu-html

[PiotrKorkus]

test should also verify saved json file

[PiotrKorkus]

nitpick:

  info!(
        instance = field::debug(kvs1.parameters().instance_id),
        key = logic.key, 
        value = value1
    );

imho better to log real istance id, custom name to distinguish between two sounds too similar and can be misleading