[task_02] Labwork2#362
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds modular testing for labwork 2, implementing unit tests using Google Test framework for mathematical model functions (linear and non-linear). The submission includes test code, implementation files, build configuration, and documentation with test results and code coverage metrics.
Key Changes
- Added unit tests for linear and non-linear mathematical model functions using Google Test
- Implemented two mathematical model functions (linear and non_linear) with corresponding header file
- Configured CMake build system with Google Test integration
- Provided documentation with test results and code coverage analysis
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| trunk/ii02824/task_02/test/test.cpp | Implements 6 unit tests covering basic cases, zero coefficients, negative inputs, and sinusoidal effects for both linear and non-linear models |
| trunk/ii02824/task_02/src/task_01.h | Declares function prototypes for linear and non_linear mathematical model functions |
| trunk/ii02824/task_02/src/task_01.cpp | Implements the linear and non_linear model functions with mathematical formulas |
| trunk/ii02824/task_02/src/CMakeLists.txt | Configures CMake build with Google Test framework integration and test discovery |
| trunk/ii02824/task_02/doc/readme.md | Documents the lab work with student information, test code, results, and coverage metrics |
| trunk/ii02824/task_02/doc/images/img1.png | Binary image file showing test execution results |
| trunk/ii02824/task_02/doc/images/img2.png | Binary image file showing code coverage analysis |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,53 @@ | |||
| #define _USE_MATH_DEFINES | |||
| #include <gtest/gtest.h> | |||
| #include "../src/task_01.h" | |||
There was a problem hiding this comment.
The include path references "task_01.h" but the directory is named "task_02". While this works because the file is in the src directory, this creates confusion. Consider renaming the header and source files to match the task number (task_02) for consistency.
| #include "../src/task_01.h" | |
| #include "../src/task_02.h" |
| double non_linear(double y, double& y_p, double u, double a, double b, double c, double d) | ||
| { | ||
| y_p = y; | ||
| return a * y - b * y_p * y_p + c * u + d * std::sin(u); |
There was a problem hiding this comment.
The non_linear function implementation has a bug. The variable y_p is assigned the value of y on line 10, but then line 11 uses y_p in the calculation (b * y_p * y_p) instead of using y. This means the function computes "a * y - b * y * y" instead of potentially using a previous value of y_p. The test on line 32 expects "b * y * y", which happens to match this implementation, but the parameter y_p should be an input representing the previous value, not just assigned from y. This creates ambiguity about the intended behavior.
| return a * y - b * y_p * y_p + c * u + d * std::sin(u); | |
| return a * y - b * y * y + c * u + d * std::sin(u); |
| #pragma once | ||
|
|
||
| double linear(double y, double u, double a, double b); | ||
| double non_linear(double y, double& y_p, double u, double a, double b, double c, double d); No newline at end of file |
There was a problem hiding this comment.
The file is named "task_01.h" but is located in the task_02 directory. This naming inconsistency could cause confusion. Consider renaming to "task_02.h" to match the task directory.
| #include <cmath> | ||
|
|
||
| double linear(double y, double u, double a, double b) | ||
| { | ||
| return a * y + b * u; | ||
| } | ||
|
|
||
| double non_linear(double y, double& y_p, double u, double a, double b, double c, double d) | ||
| { | ||
| y_p = y; | ||
| return a * y - b * y_p * y_p + c * u + d * std::sin(u); | ||
| } No newline at end of file |
There was a problem hiding this comment.
The file is named "task_01.cpp" but is located in the task_02 directory. This naming inconsistency could cause confusion. Consider renaming to "task_02.cpp" to match the task directory.
| <br><br><br><br><br> | ||
| <p align="right">Выполнил:</p> | ||
| <p align="right">Студент 2 курса</p> | ||
| <p align="right">Группы ИИ-28</p> |
There was a problem hiding this comment.
The documentation refers to "Группы ИИ-28" (Group II-28) but the directory path indicates ii02824. While the student may be from group II-28, this inconsistency should be verified to ensure the documentation matches the actual student identifier.
| <p align="right">Группы ИИ-28</p> | |
| <p align="right">Группы ИИ-28 (ii02824)</p> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| double linear(double y1, double u1, double a1, double b1); | ||
| double non_linear(double y1, double& y1_p1, double u1, double a1, double b1, double c1, double d1); No newline at end of file |
There was a problem hiding this comment.
Parameter names use a '1' suffix (e.g., y1, u1, a1) which is unnecessary and reduces readability. Consider renaming to more conventional names like y, u, a, b, etc., which would match the test code and improve clarity.
| double linear(double y1, double u1, double a1, double b1); | |
| double non_linear(double y1, double& y1_p1, double u1, double a1, double b1, double c1, double d1); | |
| double linear(double y, double u, double a, double b); | |
| double non_linear(double y, double& y_p1, double u, double a, double b, double c, double d); |
| @@ -0,0 +1,35 @@ | |||
| cmake_minimum_required(VERSION 3.14) | |||
| project(my_project) | |||
There was a problem hiding this comment.
The project name 'my_project' is generic and non-descriptive. Consider using a more meaningful name like 'task_02_models' or 'labwork2' that reflects the actual purpose of the project.
| project(my_project) | |
| project(task_02_models) |
| my_tests1 | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/task_01.cpp | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/../test/test.cpp | ||
| ) | ||
|
|
||
| target_include_directories(my_tests1 PRIVATE | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/../src | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/../test | ||
| ) | ||
|
|
||
| target_link_libraries( | ||
| my_tests1 | ||
| GTest::gtest_main | ||
| ) | ||
|
|
||
| include(GoogleTest) | ||
| gtest_discover_tests(my_tests1) No newline at end of file |
There was a problem hiding this comment.
The executable name 'my_tests1' is non-descriptive. Consider a more meaningful name like 'model_tests' or 'task_01_tests' that better describes what is being tested.
| my_tests1 | |
| ${CMAKE_CURRENT_SOURCE_DIR}/task_01.cpp | |
| ${CMAKE_CURRENT_SOURCE_DIR}/../test/test.cpp | |
| ) | |
| target_include_directories(my_tests1 PRIVATE | |
| ${CMAKE_CURRENT_SOURCE_DIR}/../src | |
| ${CMAKE_CURRENT_SOURCE_DIR}/../test | |
| ) | |
| target_link_libraries( | |
| my_tests1 | |
| GTest::gtest_main | |
| ) | |
| include(GoogleTest) | |
| gtest_discover_tests(my_tests1) | |
| task_01_tests | |
| ${CMAKE_CURRENT_SOURCE_DIR}/task_01.cpp | |
| ${CMAKE_CURRENT_SOURCE_DIR}/../test/test.cpp | |
| ) | |
| target_include_directories(task_01_tests PRIVATE | |
| ${CMAKE_CURRENT_SOURCE_DIR}/../src | |
| ${CMAKE_CURRENT_SOURCE_DIR}/../test | |
| ) | |
| target_link_libraries( | |
| task_01_tests | |
| GTest::gtest_main | |
| ) | |
| include(GoogleTest) | |
| gtest_discover_tests(task_01_tests) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| double linear(double y1, double u1, double a1, double b1); | ||
| double non_linear(double y1, double& y1_p1, double u1, double a1, double b1, double c1, double d1); No newline at end of file |
There was a problem hiding this comment.
The variable names in the header file use inconsistent suffixes. The parameters are named y1, y1_p1, u1, a1, b1, c1, d1, but based on the context and usage in tests, these should represent generic parameters, not specifically "1"-indexed values. Consider using more descriptive names like 'y', 'y_prev', 'u', 'a', 'b', 'c', 'd' to avoid confusion with the test variable naming convention.
| double linear(double y1, double u1, double a1, double b1); | |
| double non_linear(double y1, double& y1_p1, double u1, double a1, double b1, double c1, double d1); | |
| double linear(double y, double u, double a, double b); | |
| double non_linear(double y, double& y_prev, double u, double a, double b, double c, double d); |
|
No description provided.