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

Add Prefix in TestContext.AddTestAttachment for long file paths #4665

Merged
merged 4 commits into from
Mar 19, 2024

Conversation

Meet2rohit99
Copy link
Contributor

@Meet2rohit99 Meet2rohit99 commented Mar 16, 2024

Fixes #4353.

For .NetFramework, with file path greater than 260, TestContext.AddTestAttachment was throwing attachment not found exception. As suggested prefixing "\\?\" with absolute path works fine. The issue is with .NET Framework variants but not on .NET Core3.1, .NET6, or .NET7. Therefore adding prefix only in case of .NetFramework

              string prefix = string.Empty;
  #if NETFRAMEWORK
              if (filePath.Length > 260)
                  prefix = @"\\?\";
  #endif
              if (!File.Exists($"{prefix}{filePath}"))
                  throw new FileNotFoundException("Test attachment file path could not be found.", filePath);

@Meet2rohit99
Copy link
Contributor Author

@dotnet-policy-service agree

@dotnet-policy-service agree

@stevenaw
Copy link
Member

Thanks for your contribution @Meet2rohit99
The changes themselves look great! Would you be able to add a test for your change too?

@Meet2rohit99
Copy link
Contributor Author

Thanks for your contribution @Meet2rohit99 The changes themselves look great! Would you be able to add a test for your change too?

Thanks @stevenaw! Added test.

Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

See comments in PR.

src/NUnitFramework/tests/TestContextTests.cs Outdated Show resolved Hide resolved
src/NUnitFramework/framework/TestContext.cs Show resolved Hide resolved
src/NUnitFramework/tests/TestContextTests.cs Outdated Show resolved Hide resolved
Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

Thanks. Looking good now. Builds are all green.

Copy link
Member

@stevenaw stevenaw left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @Meet2rohit99 ! This looks good to me too

@stevenaw stevenaw merged commit 2f9d052 into nunit:master Mar 19, 2024
5 checks passed
@Meet2rohit99 Meet2rohit99 deleted the Issue4353 branch March 19, 2024 07:18
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.

TestContext.AddTestAttachment with long file paths
3 participants