[SERVER-63554] BackupBlock filepaths and filenames improperly mixed Created: 10/Feb/22  Updated: 15/Sep/23

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Matt Kneiser Assignee: Backlog - Storage Execution Team
Resolution: Unresolved Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Storage Execution
Operating System: ALL
Participants:

 Description   

1. BackupCursor Unit Tests use BackupBlock interface improperly (functional)

Some unit tests in the enterprise repo improperly pass in a filename stem instead of a filepath to the BackupBlock() constructor. Much of this code is brittle. Some examples:

  • TestTypicalCursorLifetime (backup_cursor_service_test.cpp)
  • TestFilenameCheck (document_source_backup_cursor_test.cpp)

 

2. _forTest & _ForTest naming is inconsistent (style)

standardize on _forTest which is more prevalent

src/fcbis/file_copy_based_initial_syncer.h

src/fcbis/file_copy_based_initial_syncer_test.cpp

  • getBackupCursorFiles_ForTest
  • getExtendedBackupCursorFiles_ForTest
  • setOldStorageFilesToBeDeleted_ForTest
    • corollary: getOldStorageFilesToBeDeleted_forTest

 



 Comments   
Comment by Matt Kneiser [ 06/Jun/23 ]

cc: gregory.wlodarek@mongodb.com 

Comment by Matt Kneiser [ 05/Jun/23 ]

The poor class interface design means tests are left to their own devices to check filenames via strings that are technically paths. I would recommend having both a filename and filepath interface on this class OR strip the path at the ctor and only deal with filenames (as isRequired does).

Comment by Matt Kneiser [ 05/Jun/23 ]

SERVER-65974 touched some of this code. I still think #1 is a functional bug in the interface that should be fixed. We should store internally a boost::filesystem::path and return a string via .filename(). We are currently mixing filepath's and names with no way to distinguish.

The second part is a simple style bug.

I revise my description of this ticket - it's a bug in the BackupBlock class, not its usage in the tests.

Comment by Connie Chen [ 05/Jun/23 ]

matt.kneiser@mongodb.com - Is the test not covering the use cases we wanted to?

Generated at Thu Feb 08 05:58:04 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.