Skip to content
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

More array.h and field.h tests #159

Merged
merged 16 commits into from
Nov 16, 2022
Merged

More array.h and field.h tests #159

merged 16 commits into from
Nov 16, 2022

Conversation

willGraham01
Copy link
Collaborator

@willGraham01 willGraham01 commented Nov 2, 2022

Mentioning #165

A number of these classes rely on methods in matlabio.h. Towards the end of this list, I stopped writing near-identical tests for the constructors/initialise() methods of these classes, because this functionality can be checked with unit tests on the aforementioned file. Additionally, any methods that use matlabio.h be redundant when #70 is completed.

DispersiveMultiLayer tests | Closes #158

  • Tests the constructor throws an error when passed something that isn't a struct
  • Tests the constructor throws an error when passed an empty struct array
  • Tests that data can be correctly assigned to the various fields of the created structure array

FrequencyVector tests | Closes #161

  • Tests that attributes initially contain nullptrs
  • initialise(): Tests that providing a pointer to an empty array exits
  • initialise(): Tests that providing a struct with too few/many fields, or an array that isn't a struct, throws an error
  • initialise(): Tests that providing a struct of the correct fields can have its data copied into the FrequencyVector class

DTilde tests | Closes #162

  • Tests attributes are initially zero and nullptr as appropriate
  • initialise(): Tests that providing a pointer to an empty array exits
  • initialise(): Tests that providing a struct with too few/many fields, or an array that isn't a struct, throws an error
  • initialise(): Tests that providing a struct with the correct fields can have its data pointed to in the DTilde {x,y} attributes

IncidentField tests | Closes #163

  • Tests that providing a pointer to an empty array throws a construction error
  • Tests that providing a struct with too few/many fields, or an array that isn't a struct, throws an error in construction
  • Tests that providing a struct with the correct fields can have its data pointed to in the IncidentField {x,y} attributes, and this data is as expected

FieldSample tests | Closes #166

  • Tests the construction of FieldSample objects via structs with populated arrays
  • Tests that the vector values are correctly extracted into the object
  • Checks that the tensor value is created with the correct shape
  • Checks that empty vectors can be used to initialise an instance of this class with an empty tensor

DetectorSensitivityArray tests | Closes #167

  • Tests the default constructor performs as expected. This also confirms fftw_destroy's ability to handle nullptrs
  • initialise() successfully creates a block of memory of the correct size that can be written to

tdms/CMakeLists.txt Outdated Show resolved Hide resolved
@samcunliffe samcunliffe added the testing Adding or requesting more test coverage label Nov 14, 2022
@samcunliffe samcunliffe marked this pull request as ready for review November 14, 2022 10:11
Copy link
Member

@samcunliffe samcunliffe left a comment

Choose a reason for hiding this comment

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

I see what you mean about these not being thrilling.

Feels like we could write a bit more structured testing with fixtures or tester classes or something.

A sketch:

class AbsArrayTest {
    virtual bool is_empty() = 0;
    // ...

    // and actually implement these on the parent
    void test_empty_construction_fails();
    void test_invalid_construction();
    void test_too_few_fields();
    void test_too_many_fields();
    void run_all_tests() {
           test_empty_construction_fails();
           test_invalid_construction();
           test_too_few_fields();
           test_too_many_fields();
    }
}

class IncidentFieldTest : public AbsArrayTests {
    // ...
}
class FrequencyVectorsTest : public AbsArrayTests {
    // ...
}

And plop all of the class-specific setup in the concrete instance. Then just call the same run_all_the_tests method on each class.

... I've no idea how easy this would be in the catch2 framework.

tdms/tests/unit/array_tests/test_DTilde.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_DTilde.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_DTilde.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_DTilde.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_DispersiveMultiLayer.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_DispersiveMultiLayer.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_DispersiveMultiLayer.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_FieldSample.cpp Outdated Show resolved Hide resolved
@willGraham01
Copy link
Collaborator Author

I see what you mean about these not being thrilling.

Feels like we could write a bit more structured testing with fixtures or tester classes or something.

Have linked your original comment to #165 -> once this is in, (and possibly also #171, which has many of the same problems) I can make some nice classes.

@samcunliffe
Copy link
Member

Ready to merge, then?

@willGraham01 willGraham01 merged commit 43c0a17 into main Nov 16, 2022
@willGraham01 willGraham01 deleted the wgraham-more_array_tests branch November 16, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Adding or requesting more test coverage
Projects
None yet
2 participants