Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow FileInfo_Query_FileIdInformation tests on OTHERFS #256

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gwr
Copy link

@gwr gwr commented Jan 14, 2022

The test function FileInfo_Query_FileIdInformation
tries to use FSCTL_READ_FILE_USN_DATA on OTHERFS,
which is generally not going to work.
I've modified this function to only try that on
NTFS or REFS.

This could be a possible fix for microsoft/ProtocolTestFramework#35

The test function FileInfo_Query_FileIdInformation
tries to use FSCTL_READ_FILE_USN_DATA on OTHERFS,
which is generally not going to work.
I've modified this function to only try that on
NTFS or REFS.
@Dingshouqing
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@Dingshouqing Dingshouqing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We checked the file system at beginning of FileInfo_Query_FileIdInformation, so could you update your PR as below:
Modify line 40

BaseTestSite.Assume.AreNotEqual(FileSystem.FAT32, fsaAdapter.FileSystem, "File system should not be FAT32.");

something like below:

If(!(this.fsaAdapter.FileSystem == FileSystem.NTFS || this.fsaAdapter.FileSystem == FileSystem.REFS))
{
      site.Assume.Inconclusive("FileInfo_Query_FileIdInformation only supported by NTFS or REFS file system.");
}

@gwr
Copy link
Author

gwr commented Jan 18, 2022

Reporting "Inconclusive" for other than NTFS or REFS seems OK.

I'd prefer that we could test the FileIdInformation info. level on OTHERFS
(because we do actually support it) but I'm not sure how the test might
verify that the 64-bit ID returned is the correct one on OTHERFS.

@Dingshouqing
Copy link
Member

That sounds reasonable, how about updating the changes as below:

  1. Add a new property in “MS-FSA_ServerTestSuite.deployment.ptfconfig” like following:
<Property name="WhichFileSystemSupport_FileIdInformation" value="NTFS;REFS" />
  1. Upgrade line 40 to check whether current file system supports FileIdInformation, if it configured in value list then will continue the test
if (!this.fsaAdapter.TestConfig.GetProperty("WhichFileSystemSupport_FileIdInformation").Contains(this.fsaAdapter.FileSystem.ToString()))
       {
          BaseTestSite.Assume.Inconclusive("Current file system does not support FileIdInformation.");
      }

This will provide a way for OTHERFS if it supports FileIdInformation(by update following change) and wants to test.

<Property name="WhichFileSystemSupport_FileIdInformation" value="NTFS;REFS;OTHERFS" />

@Dingshouqing
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants