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

Method of visiting private members in unit test #2666

Closed
kirk0830 opened this issue Jun 23, 2023 Discussed in #2659 · 4 comments
Closed

Method of visiting private members in unit test #2666

kirk0830 opened this issue Jun 23, 2023 Discussed in #2659 · 4 comments
Assignees

Comments

@kirk0830
Copy link
Collaborator

Meet trouble when adding unit test on module_io/bessel_basis. Relevant issue #2614 .

Discussed in #2659

Originally posted by kirk0830 June 21, 2023
The writing and debugging unit test of module_io/bessel_basis are still in progress, while thanks to Zuxin we will have a brand new module (module_nao) later, therefore the unit test of bessel_basis may be not quite important if the new module is fully implemented. However, presently, during adding unit tests on private members, the use of #define private public will iteratively cause errors like:
error: ‘struct std::__cxx11::basic_stringbuf<_CharT, _Traits, _Alloc>::__xfer_bufptr ’ redeclared with different access
446 | struct __xfer_bufptrs
| ^~~~~~
which is, obviously caused by imposing #define private public onto <sstream> (for analysis with more details posted by other coder, see why cannot use #define private public anymore?). Got advice from Tianqi that removing unnecessary dependence of header files on other header files may finally solve this error, but this error pops again and again, from global_function.h to global_variable.h, then realarray.h, unitcell.h, vector3.h and matrix3.h.

Actually there are also other methods for visiting private variables and functions, but will be more or less invasive, such as gtest/gtest_prod.h::FRIEND_TEST, or the following way:

  1. first create a variable when setting BUILD_TESTING option in CMakeLists.txt is true, say __UNITTEST
  2. In header file, use
    #ifdef __UNITTEST
    #define PRIVATE_SWTICH public
    #else
    #define PRIVATE_SWTICH private
    #endif
    and substitute private with PRIVATE_SWTICH.

The use of FRIEND_TEST also requires declaration in private scope of class to test. So these two methods are more or less invasive but has pros that makes the range of variables which will be set as public during unit test controllable. Suggestions provided by ChatGPT4 are pasted here:

It looks like the error is caused due to an access level conflict for struct __xfer_bufptrs within the basic_stringbuf class, which is part of the std namespace (typically declared in the <sstream> header file). The most likely reason for this error is the #define private public directive.
While this directive can help you access private members of a class for testing purposes, it also changes the access level of everything in every file that is included after the directive, not just your own classes. This can cause conflicts with library classes, such as std::basic_stringbuf, that rely on the proper access levels for their members.
This situation is a perfect example of the risks associated with using #define private public. As I suggested earlier, safer alternatives for testing private methods are refactoring your code to make it more testable, making the test a friend of the class, or using the Google Test FRIEND_TEST macro.
As a final note, the use of #define private public is generally considered a bad practice because it breaks the encapsulation principle of object-oriented programming. Moreover, it can lead to issues like the one you're experiencing, especially in complex projects that involve multiple libraries or large codebases.
It's always better to architect your classes and methods in such a way that you can test them without needing to access private members. This typically involves designing your classes with testability in mind, for example by providing public methods that can be used to check the state of the object, or by using interfaces and dependency injection so you can provide mock implementations for testing.

Therefore comments and opinions are of need and will be appreciated about method visiting private variables and functions during unit test.

@jinzx10
Copy link
Collaborator

jinzx10 commented Jul 18, 2023

After some search, I find that access check is not invoked in explicit template instantiation, and there's already some repo/code available on github which exploit this behavior for non-invasive tests:

https://github.com/martong/access_private
https://gist.github.com/dabrahams/1528856

This behavior is stated in the following paragraph from the C++11 standdard (14.7.2, paragraph 12)
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3690.pdf

"The usual access checking rules do not apply to names used to specify explicit instantiations. [ Note: In
particular, the template arguments and names used in the function declarator (including parameter types,
return types and exception specifications) may be private types or objects which would normally not be
accessible and the template may be a member template or member function which would not normally be
accessible. — end note ]"

@jinzx10
Copy link
Collaborator

jinzx10 commented Jul 18, 2023

The tool in https://github.com/martong/access_private is simply one header file, and the usage is non-invasive, i.e., no modification to the origional source file is required. I think if testing private functions is really necessary, this might be something we can introduce (or simply learn the idea & write our own) to our unit test procedure. What are your opinions @baixiaokuang @hongriTianqi ?

The following is an example with googletest.

PS:
The test/test.cpp in https://github.com/martong/access_private contains more examples, including static member & functions, etc.

testee.h (the class under test)

#ifndef TESTEE_H_
#define TESTEE_H_

#include <iostream>

class Testee {
public:
    Testee() : num_(0) {}
    ~Testee() {};

    void set(int i) { num_ = i; }
    int get() const { return num_; }

private:
    int num_;

    void addone() { num_ += 1; }
    int times_a_plus_b(int a, int b) { return a * num_ + b; }
};
#endif

unit test source file:

#include "testee.h"
#include "../include/access_private.hpp"

#include <gtest/gtest.h>

ACCESS_PRIVATE_FUN(Testee, void(), addone);
ACCESS_PRIVATE_FUN(Testee, int(int,int), times_a_plus_b);
ACCESS_PRIVATE_FIELD(Testee, int, num_)

class Tester : public ::testing::Test {

protected:
    void SetUp();
    void TearDown();

    Testee t;
};

void Tester::SetUp() {
    t.set(0);
}

void Tester::TearDown() {
}

TEST_F(Tester, test1) {
    int n = 9;

    t.set(n);
    call_private::addone(t);
    EXPECT_EQ(t.get(), n+1);

    t.set(n);
    int a = 2;
    int b = 5;
    int res = call_private::times_a_plus_b(t, a, b);
    EXPECT_EQ(res, a*n+b);

    n = 22;
    t.set(n);
    EXPECT_EQ(access_private::num_(t), n);
}


int main(int argc, char **argv) {
    testing::InitGoogleTest(&argc, argv);
    int result = RUN_ALL_TESTS();
    return result;
}

@hongriTianqi
Copy link

hongriTianqi commented Aug 17, 2023

@jinzx10 @kirk0830 Good Idea. I think we should learn from the author, and implement our own.

@kirk0830
Copy link
Collaborator Author

Private functions have been tested indirectly by testing public functions which call those private:
They are:
readin_C4(),init_TableOne(), init_Faln().
Function allocate_C4() should be tested in unit test of ModuleBase::RealArray.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

No branches or pull requests

4 participants