Repository navigation
refactor(dfpt): replace the shared fixture base class with a free-function test tool - #8270
Open
Zanthoxylum wants to merge 2 commits into
Open
Zanthoxylum wants to merge 2 commits into
Zanthoxylum wants to merge 2 commits into
Conversation
…ction test tool
The DFPTSerialBase base class and its header are removed in favor of
dfpt_test_tool.{h,cpp} (the module_pw test/test_tool pattern): shared
declarations plus one implementation TU of free functions -- setup_cell,
make_coulomb_atom, make_nc_atom, make_two_atom_cell, make_nlcc_two_atom_
cell, setup_bases, vloc_coulomb, analytic_dvloc, fkey, gkey -- and the
shared test-world constants (lat0 / ecutwfc / rho_mult / cubic_edge).
Every test file now derives its own fixture directly from ::testing::Test,
owns its objects as members and calls the utilities it needs; there is no
shared fixture base class and no fixture header. The rho files keep their
file-local RNG. CMakeLists swaps dfpt_serial_fixture.cpp for
dfpt_test_tool.cpp in every target. setup_bases keeps the raw assembly
chains and adopts PW_Basis::init_serial once deepmodeling#8268 lands. All six serial
targets rebuilt from deleted binaries (make rc=0); case counts conserved
(pert 7 / phon 20 / rho 11 / q0 6 / xc 6 / write_dyn 6); full MODULE_DFPT
ctest 10/10.
ABACUS-PR-BOT
left a comment
Collaborator
There was a problem hiding this comment.
Static review of 68d7f3fd5 found no functional regression in the fixture extraction; two include cleanups are noted below. Builds and tests were not run.
| // its own gtest fixture from ::testing::Test, owns the objects it needs | ||
| // and calls these functions -- there is no shared fixture base class. | ||
|
|
||
| #include "source_base/matrix3.h" |
Collaborator
There was a problem hiding this comment.
Non-blocking: This declaration-only header uses Matrix3 and Vector3<double> only by reference. Forward-declare them, as you do for the other argument types, and include their definitions in the implementation and fixtures that need them.
| // per-file fixture (self-contained, gtest style): only the members and | ||
| // helpers this file's cases need | ||
| class DFPTPhonSerialTest : public DFPTSerialBase | ||
| #include "gmock/gmock.h" |
Collaborator
There was a problem hiding this comment.
Non-blocking: This newly added include block is repeated below the fixture at lines 69–74. Please keep one copy of each header here and remove the analogous duplicates in the other changed phon, pert, q0 and rho test files.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DFPTSerialBase(the shared gtest fixture base class and its header,dfpt_serial_fixture.{h,cpp}) is replaced by a free-function test tool,following the established
module_pw test/test_tool.{h,cpp}pattern:small declaration-only header + one implementation TU compiled into
every test target.
dfpt_test_tool.{h,cpp}provides the shared utilities as freefunctions —
setup_cell,make_coulomb_atom,make_nc_atom,make_two_atom_cell,make_nlcc_two_atom_cell,setup_bases,vloc_coulomb,analytic_dvloc,fkey,gkey— plus the sharedtest-world constants (
lat0,ecutwfc,rho_mult,cubic_edge).::testing::Test, owns its objects (ucell_,pw_rho_,pw_wfc_,qlist_,data_,q_cart_, ...) as members and calls exactly theutilities it needs. There is no shared fixture base class and no
fixture header anywhere in the module.
CMakeLists.txtswapsdfpt_serial_fixture.cppfordfpt_test_tool.cppin every test target.Relationship to #8268
Both PRs touch
dfpt_serial_fixture.cpp— #8268 modifiesSetupBasesinside it (to call the new
PW_Basis::init_serial); this PR deletes thefile entirely. Either merge order resolves trivially:
edit), and
setup_basesadoptsinit_serialas a two-line follow-up(the adoption point is marked in
dfpt_test_tool.cpp);module_pw API part is unaffected.
Verification
code 0 per target).
write_dyn 6 (identical case names, none added or lost).
MODULE_DFPTsuite: 10/10.Documentation
No docs change needed: test-only refactor with no INPUT, interface, or
behavior change.