Skip to content

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

Merged
stevenaw merged 4 commits into
nunit:masterfrom
Meet2rohit99:Issue4353
Mar 19, 2024
Merged

Add Prefix in TestContext.AddTestAttachment for long file paths#4665
stevenaw merged 4 commits into
nunit:masterfrom
Meet2rohit99:Issue4353

Conversation

@Meet2rohit99
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

@dotnet-policy-service agree

@dotnet-policy-service agree

@stevenaw
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread src/NUnitFramework/tests/TestContextTests.cs Outdated
Comment thread src/NUnitFramework/framework/TestContext.cs
Comment thread src/NUnitFramework/tests/TestContextTests.cs Outdated
Copy link
Copy Markdown
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
Copy Markdown
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
@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