pingcap / pingcap/tiflash

Remove embedded member function for gtest in `dbms/src/Storages/Page`

Open
#4,562 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

good first issue help wanted type/code-quality-improvement type/enhancement
Dominant language
C++
Stars
1k
Forks
423
Avg merge
1d 15h
Merged PRs (30d)
24

Description

Enhancement

There are some embedded member function in the directory dbms/src/Storages/Page
like PS::V3::PageStorageImpl

#ifndef NDEBUG
    // Just for tests, refactor them out later
    DB::PageStorage::SnapshotPtr getSnapshot() { return getSnapshot(""); }
    DB::PageEntry getEntry(PageId page_id) { return getEntryImpl(TEST_NAMESPACE_ID, page_id, nullptr); }
    DB::Page read(PageId page_id) { return readImpl(TEST_NAMESPACE_ID, page_id, nullptr, nullptr); }
    PageMap read(const std::vector<PageId> & page_ids) { return readImpl(TEST_NAMESPACE_ID, page_ids, nullptr, nullptr); }
    void read(const std::vector<PageId> & page_ids, const PageHandler & handler) { return readImpl(TEST_NAMESPACE_ID, page_ids, handler, nullptr, nullptr); }
    PageMap read(const std::vector<PageReadFields> & page_fields) { return readImpl(TEST_NAMESPACE_ID, page_fields, nullptr, nullptr); }
#endif

which is just for tests. This is not a elegant code style, and we need to remove them.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in dbms/src/Storages/Page and inspect PS::V3::PageStorageImpl, including the gtest callers of the embedded methods shown in the issue. Remove the test-only member functions and update affected tests to use the remaining interfaces; the Page storage tests should still pass without those wrappers.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases, testing
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.