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

Use hashed file contents as unique test ID #1605

Merged
merged 1 commit into from
Jul 5, 2017

Conversation

ozyx
Copy link
Contributor

@ozyx ozyx commented Jun 14, 2017

Issue #1561

Bug

TFS unable to detect the same test failing in a previous build if the path of the *.js file differs.

Since a test's path is used in its FullyQualifiedName, the same test run on two different agents with different drive names (i.e: D:/ vs. C:/ drives) will not be recognized as the same test by TFS.

Fix

Hash the contents of the test file using SHA1 (so as not to slow down test discovery significantly) and use this as a ModuleName / unique ID.

Previous FullyQualifiedId: C:\some\path\to\my\test.js::Suite 1 Test 1::Mocha
New FullyQualifiedId: test[60E439FF4023183BDC8C]::Suite 1 Test 1::Mocha

NOTE: An assumption using this method is that two test files do not exist in the same project with the exact same name and contents.

Testing
  1. Create sample test project
  2. Copy project to a different path
  3. Open both projects
  4. The same test in both projects should have the same FullyQualifiedId

This will ensure intended TFS behavior of failed test detection in current vs. previous builds.

Please let me know if there are any improvements or changes I should make. Thanks!

@paulvanbrenk
Copy link
Contributor

Overall this looks pretty good... I'm not sure cutting the hash in half, gives a random enough hash... and I'm sure we'll get flagged for using this algorithm, so we'll have to explain that we're not using it for encryption.

Would GetHashCode be good enough?

@ozyx
Copy link
Contributor Author

ozyx commented Jun 20, 2017

I'm not sure GetHashCode would suffice (I'm assuming you're referencing string.GetHashCode), as the hash needs to be unique to the file contents. Otherwise just using the file path would work fine. Is there an easier / cleaner way to hash the file contents?

@ozyx
Copy link
Contributor Author

ozyx commented Jun 21, 2017

Another way I could think to solve this problem would be:

  • Use filepath relative to the solution or project location
    • However, the executing binary doesn't know this info so it seems it would not be so straightforward...

Let me know your thoughts on this. I'm not sure how big of an inconvenience it is getting flagged for crypto, but it seems hashing the file contents is the simplest way to get an ID unique to the test file itself.

@paulvanbrenk
Copy link
Contributor

Yeah, I'm thinking I should just take it as is for 1.3 and see if we can come up with something else for 1.4. I'll take another look this week and either merge it or explain any changes I want.

Thanks!

@ozyx
Copy link
Contributor Author

ozyx commented Jul 1, 2017

Hi guys-- just wondering if you've had a chance to review this yet? We are using a private build internally currently and so far, so good. But would be nice to get an official release.
Thanks for your time!

@paulvanbrenk paulvanbrenk merged commit aff39e9 into microsoft:v1.3.x Jul 5, 2017
@paulvanbrenk
Copy link
Contributor

I'll get a build out later today.

@ozyx
Copy link
Contributor Author

ozyx commented Jul 5, 2017

Great, thanks!

@paulvanbrenk
Copy link
Contributor

Here you go, please let me know if you encounter any issues.

https://github.com/Microsoft/nodejstools/releases/tag/v1.3.1

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.

3 participants