New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 848403 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Refactor OpenAddedFile in SmbProviderTest to use FakeSambaInterface instead of SmbProvider

Project Member Reported by allenvic@chromium.org, May 31 2018

Issue description

Currently, OpenAddedFile in smbprovider_test.cc uses SmbProvider (variable smbprovider_) to open a file in a given path. We want to refactor this function to use FakeSambaInterface (variable fake_samba_) instead so we can get stop creating an unnecessary protobuf and also remove the mount_id parameter in the function.

Things that will be changing:
1. Use FakeSambaInterface#OpenFile instead of SmbProvider#OpenFile.
2. Remove mount_id parameter from OpenAddedFile and other calling methods.

Thing to note:
OpenAddedFile currently takes in a |path| variable, which is a relative path. With this change, it will need to take in a full path since FakeSambaInterface expects full path to the entries. There are helper methods such as GetDefaultFullPath() that converts a relative path and prepends the default mount root to the beginning of the path

Test with:
cros_workon_make --board=${BOARD} smbprovider --test

 
adokar@ spotted that CloseFileHelper in smbprovider_test.cc can also be updated to just use FakeSambaInterface#CloseFile

Comment 2 by adokar@google.com, Jun 1 2018

Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e857bb4c5d48bb5858b36e4eb268be2ac13d60aa

commit e857bb4c5d48bb5858b36e4eb268be2ac13d60aa
Author: Adolfo Victoria <adokar@google.com>
Date: Fri Jun 08 06:33:13 2018

smbprovider: Refactor tests to use fake_samba

Change OpenAddedFile and CloseFileHelper to use fake_samba_
instead of smbprovider_. This avoids making an unnecessary
protobuffer and passing mount_id.

BUG= chromium:848403 
TEST=Ran smbprovider unit tests
Change-Id: I0da9f9fea360534ff332ad926b0e481c59a5ff0d
Reviewed-on: https://chromium-review.googlesource.com/1083652
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/e857bb4c5d48bb5858b36e4eb268be2ac13d60aa/smbprovider/smbprovider_helper.cc
[modify] https://crrev.com/e857bb4c5d48bb5858b36e4eb268be2ac13d60aa/smbprovider/smbprovider_helper.h
[modify] https://crrev.com/e857bb4c5d48bb5858b36e4eb268be2ac13d60aa/smbprovider/smbprovider_helper_test.cc
[modify] https://crrev.com/e857bb4c5d48bb5858b36e4eb268be2ac13d60aa/smbprovider/smbprovider_test.cc

Comment 4 by adokar@google.com, Jun 14 2018

Status: Fixed (was: Started)

Sign in to add a comment