Skip to content

Commit 9cbcc0b

Browse files
Critsium-xyclaude
andauthored
source_cell: replace #define private public with named friend grants in four tests (#7949)
`magnetism_test`, `atom_pseudo_test`, `pseudo_nc_test` and `read_pp_test` each switched off access control for their whole translation unit in order to call a handful of private *methods* -- the format readers and helpers the tests exist to exercise. No private data member is involved, and none of these files touches PARAM, so the macro had exactly one cause. Replace it with an explicit `friend class <fixture>;` on the class under test, following the existing precedent in source_pw/module_pwdft/dftu_base.h (`friend class DFTUTest;`) and parameter.h (`friend class TestParameters;`). This is the fix AGENTS.md rule 10 names, and it narrows a whole-TU override -- which also reinterprets access control inside every standard library header the TU pulls in -- down to one named, reviewable grant per fixture. Friendship is not inherited, and a TEST_F body lives in a class derived from the fixture, so each fixture gains thin forwarding wrappers and the TEST_F bodies call those. dftu_lcao_test.cpp already documents and uses this arrangement. - Magnetism: friend MagnetismTest, 1 wrapper (judge_parallel), 2 call sites. - Pseudopot_upf: friend AtomPseudoTest / NCPPTest / ReadPPTest; 1 + 3 + 8 wrappers for read_pseudo_upf{,201,_vwr,_blps}, set_pseudo_type, setqfnew, trim, trimend, complete_default_{h,atom}; 36 call sites in total. Public members and public methods the tests already used -- Pseudopot_upf's complete_default, init_pseudo_reader, average_p, set_upf_q, set_empty_element, print_pseudo_upf and its public data, and Magnetism's tot_mag/abs_mag/start_mag -- are untouched and still accessed directly. No test expectation changed: every wrapper forwards its arguments unmodified and returns what the private method returns. Macro occurrences in these four files go 4 -> 0; no `#undef private` is introduced anywhere, and no file whose macro survives is touched. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent fe3949f commit 9cbcc0b

6 files changed

Lines changed: 127 additions & 46 deletions

File tree

source/source_cell/magnetism.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,16 @@
55
#include "source_base/vector3.h"
66
#include <vector>
77

8+
class MagnetismTest;
9+
810
/**
911
* @brief Class for magnetism calculations.
1012
*/
1113
class Magnetism
1214
{
15+
/// @brief the unit test drives the private judge_parallel() helper directly
16+
friend class MagnetismTest;
17+
1318
public:
1419
/// @brief Constructor
1520
Magnetism();

source/source_cell/read_pp.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111
#include "source_base/matrix.h"
1212
#include "source_base/realarray.h"
1313

14+
class AtomPseudoTest;
15+
class NCPPTest;
16+
class ReadPPTest;
17+
1418
/**
1519
* @brief Pseudopot_upf class for reading pseudopotential files.
1620
*
@@ -19,6 +23,12 @@
1923
*/
2024
class Pseudopot_upf
2125
{
26+
/// @brief the unit tests drive the private format readers and the
27+
/// complete_default_* helpers directly; see source_cell/test/
28+
friend class AtomPseudoTest;
29+
friend class NCPPTest;
30+
friend class ReadPPTest;
31+
2232
public:
2333
/// PP_INFO
2434
/// PP_HEADER

source/source_cell/test/atom_pseudo_test.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,22 @@
2222
* - bcast upf201 pp info to other processes
2323
*/
2424

25-
#define private public
2625
#include "source_cell/read_pp.h"
2726
#include "source_cell/pseudo.h"
2827
#include "source_cell/atom_pseudo.h"
29-
#undef private
3028
class AtomPseudoTest : public testing::Test
3129
{
3230
protected:
3331
std::unique_ptr<Pseudopot_upf> upf{new Pseudopot_upf};
3432
std::unique_ptr<Atom_pseudo> atom_pseudo{new Atom_pseudo};
33+
34+
// Pseudopot_upf declares this fixture a friend, but a TEST_F body lives in
35+
// a class derived from it, and friendship is not inherited -- so the call
36+
// into the private format reader has to happen here.
37+
int read_pseudo_upf201(std::ifstream& ifs, Atom_pseudo& pp) const
38+
{
39+
return upf->read_pseudo_upf201(ifs, pp);
40+
}
3541
};
3642

3743
TEST_F(AtomPseudoTest, SetDSo)
@@ -43,7 +49,7 @@ TEST_F(AtomPseudoTest, SetDSo)
4349
std::ifstream ifs;
4450
ifs.open("./support/C.upf");
4551
const double pseudo_rcut = 15.0;
46-
upf->read_pseudo_upf201(ifs, *atom_pseudo);
52+
read_pseudo_upf201(ifs, *atom_pseudo);
4753
upf->complete_default(*atom_pseudo, pseudo_rcut);
4854
ifs.close();
4955
EXPECT_EQ(atom_pseudo->nh,14);
@@ -75,7 +81,7 @@ TEST_F(AtomPseudoTest, BcastAtomPseudo)
7581
std::ifstream ifs;
7682
ifs.open("./support/C.upf");
7783
const double pseudo_rcut = 15.0;
78-
upf->read_pseudo_upf201(ifs, *atom_pseudo);
84+
read_pseudo_upf201(ifs, *atom_pseudo);
7985
upf->complete_default(*atom_pseudo, pseudo_rcut);
8086
ifs.close();
8187
}

source/source_cell/test/magnetism_test.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
* - and non-collinear case with nspin = 4
1818
*/
1919

20-
#define private public
2120
#include "source_cell/magnetism.h"
22-
#undef private
2321

2422

2523
class MagnetismTest : public ::testing::Test
@@ -34,6 +32,14 @@ class MagnetismTest : public ::testing::Test
3432
{
3533
delete magnetism;
3634
}
35+
36+
// Magnetism declares this fixture a friend, but a TEST_F body lives in a
37+
// class derived from it, and friendship is not inherited -- so the call
38+
// into the private helper has to happen here.
39+
bool judge_parallel(const double a[3], const ModuleBase::Vector3<double>& b) const
40+
{
41+
return magnetism->judge_parallel(a, b);
42+
}
3743
};
3844

3945
TEST_F(MagnetismTest, Magnetism)
@@ -47,9 +53,9 @@ TEST_F(MagnetismTest, JudgeParallel)
4753
{
4854
double a[3] = {1.0, 0.0, 0.0};
4955
ModuleBase::Vector3<double> b(1.0, 0.0, 0.0);
50-
EXPECT_TRUE(magnetism->judge_parallel(a, b));
56+
EXPECT_TRUE(judge_parallel(a, b));
5157
b = ModuleBase::Vector3<double>(0.0, 1.0, 0.0);
52-
EXPECT_FALSE(magnetism->judge_parallel(a, b));
58+
EXPECT_FALSE(judge_parallel(a, b));
5359
}
5460

5561
TEST_F(MagnetismTest, ComputeMagnetizationS2)

source/source_cell/test/pseudo_nc_test.cpp

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,39 @@
2121
* - print_pseudo
2222
*/
2323

24-
#define private public
2524
#include "source_cell/read_pp.h"
2625
#include "source_cell/atom_pseudo.h"
27-
#undef private
2826
class NCPPTest : public testing::Test
2927
{
3028
protected:
3129
std::unique_ptr<Pseudopot_upf> upf{new Pseudopot_upf};
3230
std::unique_ptr<Atom_pseudo> ncpp{new Atom_pseudo};
31+
32+
// Pseudopot_upf declares this fixture a friend, but a TEST_F body lives in
33+
// a class derived from it, and friendship is not inherited -- so the calls
34+
// into the private reader and the complete_default_* helpers happen here.
35+
int read_pseudo_upf201(std::ifstream& ifs, Atom_pseudo& pp) const
36+
{
37+
return upf->read_pseudo_upf201(ifs, pp);
38+
}
39+
void complete_default_h(Atom_pseudo& pp) const
40+
{
41+
upf->complete_default_h(pp);
42+
}
43+
void complete_default_atom(Atom_pseudo& pp, const double pseudo_rcut) const
44+
{
45+
upf->complete_default_atom(pp, pseudo_rcut);
46+
}
3347
};
3448

3549
TEST_F(NCPPTest, SetPseudoH)
3650
{
3751
std::ifstream ifs;
3852
//set
3953
ifs.open("./support/C.upf");
40-
upf->read_pseudo_upf201(ifs, *ncpp);
54+
read_pseudo_upf201(ifs, *ncpp);
4155
//set_pseudo_h
42-
upf->complete_default_h(*ncpp);
56+
complete_default_h(*ncpp);
4357

4458
if(!ncpp->has_so)
4559
{
@@ -62,10 +76,10 @@ TEST_F(NCPPTest, SetPseudoAtom)
6276
//set
6377
ifs.open("./support/C.upf");
6478
const double pseudo_rcut = 15.0;
65-
upf->read_pseudo_upf201(ifs, *ncpp);
79+
read_pseudo_upf201(ifs, *ncpp);
6680
//set_pseudo_atom
67-
upf->complete_default_h(*ncpp);
68-
upf->complete_default_atom(*ncpp, pseudo_rcut);
81+
complete_default_h(*ncpp);
82+
complete_default_atom(*ncpp, pseudo_rcut);
6983
EXPECT_EQ(ncpp->rcut,pseudo_rcut);
7084

7185
if(!ncpp->nlcc)
@@ -86,12 +100,12 @@ TEST_F(NCPPTest, SetPseudoNC)
86100
ifs.open("./support/C.upf");
87101
const double pseudo_rcut = 15.0;
88102
// set pseudo nbeta = 0
89-
upf->read_pseudo_upf201(ifs, *ncpp);
103+
read_pseudo_upf201(ifs, *ncpp);
90104
ncpp->nbeta = 0;
91105
upf->complete_default(*ncpp, pseudo_rcut);
92106
EXPECT_EQ(ncpp->nh,0);
93107
// set pseudo nbeta > 0
94-
upf->read_pseudo_upf201(ifs, *ncpp);
108+
read_pseudo_upf201(ifs, *ncpp);
95109
upf->complete_default(*ncpp, pseudo_rcut);
96110
EXPECT_EQ(ncpp->nh,14);
97111
EXPECT_EQ(ncpp->kkbeta,132);
@@ -105,7 +119,7 @@ TEST_F(NCPPTest, PrintNC)
105119
//set
106120
ifs.open("./support/C.upf");
107121
const double pseudo_rcut = 15.0;
108-
upf->read_pseudo_upf201(ifs, *ncpp);
122+
read_pseudo_upf201(ifs, *ncpp);
109123
upf->complete_default(*ncpp, pseudo_rcut);
110124
ifs.close();
111125
//print

0 commit comments

Comments
 (0)