-
Notifications
You must be signed in to change notification settings - Fork 21
Test Cases for default values in case of CPP added #171
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
Test Cases for default values in case of CPP added #171
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
f484ec1 to
3e5a5c6
Compare
|
The created documentation from the pull request is available at: docu-html |
6e98d14 to
f011478
Compare
|
Run locally |
97ec258 to
5548a2e
Compare
|
Please restore and use 4 space indentation for cpp files |
ec4cde8 to
1b1d2b6
Compare
pahmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am missing any information for the test properties on which requirements they apply to and which test method was used. They are supposed to be Component Integration Tests, so we should be able to find a description as mentioned here: https://github.com/eclipse-score/process_description/blob/main/process/process_areas/verification/guidance/verification_templates.rst#c-properties-template
| reset_single_key_scenario, checksum_scenario}, | ||
| {}}}; | ||
|
|
||
| ScenarioGroup::Ptr cit_group{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only Component Integratio Tests are expected here AFAIK, so we can remove this additional group.
| std::cerr << ex.what() << std::endl; | ||
| return 1; | ||
| run_cli_app(raw_arguments, test_context); | ||
| } catch (const ScenarioError &ex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if we want try catch around run_cli_app at all.
Right now there is just a description string that you still need to find manually or add some debug steps.
Without try catch there will be termination like:
terminate called after throwing an instance of 'std::runtime_error'
what(): TestError
and you just need to rerun it with gdb with bt at the end to find where it happened.
To be discussed.
|
Partial review done, have not touched test implementation yet. |
Hi @pahmann, the properties are added in python file: |
| * SPDX-License-Identifier: Apache-2.0 | ||
| ********************************************************************************/ | ||
| #ifndef TEST_DEFAULT_VALUES_HPP | ||
| #define TEST_DEFAULT_VALUES_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Header guard or #pragma once, choose one. I would go for the pragma version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When choosing one, Does the header guard does not give more portability?
In my knowledge the pragma is not supported by a few compilers.
| * | ||
| * @param key The key being queried or modified in the KVS. | ||
| * @param value_is_default String encoding whether the current value matches the | ||
| * default ("Ok(true)", "Ok(false)", or error string). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see now that we actually have python part with Ok(), Err() and lots of stringification... It should be refactored to have there original values. But it will be done as some future task, now let's keep it as is for now.
| << std::get<double>(get_value_result.value().getValue()); | ||
| current_value = "Ok(F64(" + oss.str() + "))"; | ||
| } | ||
| // value_is_default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In rust there is a special function to determine if given value is default or set value. In a scenario where default value is set to x and then the key is overwritten with regular value x this WorkAround would still show that is default value. Rust implementation would return that is later set value.
I would contact cpp persistency devs to determine if they are going to fix the gap to understand where it is going to and maybe just use has_default_value for now with eventual xfail.
That applies to many other occurrences of this issue - we can discuss.
5c92761 to
b148d5b
Compare
8530283 to
d8a875c
Compare
* Use docs_as_code 2.0.1 * Use score_platform 0.4.1 * external needs are necessary when using persistency, they cannot be dev_dependency Co-authored-by: Alexander Lanin <Alexander.Lanin@etas.com>
Added missing copyright check and copyright info to some files. Signed-off-by: Nicolae Dicu <nicolae.dicu.ext@qorix.ai>
- `KvsSerialize` and `KvsDeserialize` traits. - Basic types support. - Unit tests for basic types serialization/deserialization. - Use-case examples.
Replaced `Option` with `SerializationFailed` and `DeserializationFailed` errors, containing string with failure reason.
- Require hash file for default files. - Make hash parameter non-optional. - Update tests.
- moving code to namespace - add empty lines - remove default value vector - move comparison to function - added enum for FAILURE result code adding spaces to comments
d8a875c to
effcf28
Compare
|
A new PR is created to clean and address the issues. |
This PR adds C++ test cases for default values scenarios to achieve parity with existing Rust tests. The implementation includes a temporary workaround to create hash files for C++ KVS, which requires them for validation (unlike Rust). The changes enable parameterized testing across both Rust and C++ versions.
Key changes:
Added C++ test scenarios for default values testing (default_values, remove_key, reset operations, checksum)
Implemented temporary hash file generation in Python test fixtures to support C++ requirements
Modified Rust test scenarios to use string-based result logging for cross-language compatibility
The CPP KVS needs a hash file which is not needed by RUST , leading to failure of test cases.
Created the issue at : #170