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

added DeploymentItemExAttribute, much like DeploymentItem, but supports long file paths #3

Merged
merged 4 commits into from
Mar 29, 2013

Conversation

icnocop
Copy link
Contributor

@icnocop icnocop commented Mar 26, 2013

Notice there are two "// TODO:" items.

  1. The ability to output warnings to the test results log file.

Is this possible?

  1. The ability to get the value of the RelativePathRoot property setting in the current .testsettings file

See this msdn article for more information:
http://msdn.microsoft.com/en-us/library/ms182475%28v=vs.100%29.aspx
"Relative paths are relative to the RelativePathRoot setting found in the .testrunconfig file."

I could not personally find this specific setting, so I'm not sure if this is just a documentation error. I've also read that it is set to the solution directory by default, so that is what I am trying to use for now.

Once this new attribute looks good, I can go ahead and try to add unit\integration tests for it.

What do you think?

Thank you.

@icnocop
Copy link
Contributor Author

icnocop commented Mar 26, 2013

See related discussion: http://mstestextensions.codeplex.com/discussions/437712

@CallumHibbert
Copy link
Member

What does this give us over using the ".testsettings" and adding items under the "Deployment" section?

Also, I not personally had any issues with the built in "DeploymentItem" attribute, I'm not saying there aren't any issues with it but I wondered what problem we are solving with this new attribute.

Thanks.

@icnocop
Copy link
Contributor Author

icnocop commented Mar 26, 2013

The DeploymentItem attribute allows us to specify the deployment items that are only specific to each test for example. In this way, the external resources are coupled with the tests that require them which makes it easier to manage in the scenarios where tests are changed or removed, the deployment items are as close to the tests that use them as possible. The DeploymentItem can be used on both test classes and methods.

As noted in DeploymentItemExAttribute.cs:
DeploymentItemEx, much like DeploymentItem, but supports long file paths.

DeploymentItemEx allows source and destination deployment items to have long file paths (greater than 256 characters).

I remember creating a Microsoft connect bug on the issue but I can't find it anymore because they archived a lot of their "old" bug reports. I remember that the issue was marked as Closed with either "Won't Fix" or "By Design", etc.

@icnocop
Copy link
Contributor Author

icnocop commented Mar 26, 2013

@CallumHibbert
Copy link
Member

OK, sounds good. Please can you do two things:

1 - Rename "DeploymentItemExAttribute" to "DeploymentItemWithLongPathAttribute" as that's a more descriptive name.

2 - We are introducing a dependency on "ZetaLongPaths", so can you please add that as a dependent package in the NuGet package definition at "Source\Artefacts\NuGet\StealFocus.MSTestExtensions\StealFocus.MSTestExtensions.nuspec".

e.g.

<dependencies>
  <dependency id="ZetaLongPaths" version="1.0.0" />
</dependencies>

Once this is done, I'll merge your changes and push a new build to NuGet.

Thanks.

added ZetaLongPaths 1.0.0 as a dependency to the nuget specification
@icnocop
Copy link
Contributor Author

icnocop commented Mar 28, 2013

Hi Callum,

Not sure if you got notified on the last commit, but just wanted to let you know that I've added a commit to resolve the issues you addressed.

Thank you.

@CallumHibbert CallumHibbert merged commit 4491386 into StealFocus:master Mar 29, 2013
@CallumHibbert
Copy link
Member

Rami,

No, I don't get notified of the updated pull request. Have merged your
changes just now and there is a new version on NuGet.

If you get the chance, it would be great to have a unit test for this.

Thanks.

On 28 March 2013 20:08, Rami [email protected] wrote:

Hi Callum,

Not sure if you got notified on the last commit, but just wanted to let
you know that I've added a commit to resolve the issues you addressed.

Thank you.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3#issuecomment-15611484
.

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