Use hashed file contents as unique test ID by ozyx · Pull Request #1605 · microsoft/nodejstools · GitHub
Skip to content

Use hashed file contents as unique test ID#1605

Merged
paulvanbrenk merged 1 commit into
microsoft:v1.3.xfrom
ozyx:mocha-unique-id
Jul 5, 2017
Merged

Use hashed file contents as unique test ID#1605
paulvanbrenk merged 1 commit into
microsoft:v1.3.xfrom
ozyx:mocha-unique-id

Conversation

@ozyx

@ozyx ozyx commented Jun 14, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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!

@ozyx ozyx mentioned this pull request Jun 14, 2017
Closed
@paulvanbrenk

paulvanbrenk commented Jun 20, 2017

Copy link
Copy Markdown
Contributor

@ozyx

ozyx commented Jun 20, 2017

Copy link
Copy Markdown
Contributor Author

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

ozyx commented Jun 21, 2017

Copy link
Copy Markdown
Contributor Author

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

paulvanbrenk commented Jun 21, 2017

Copy link
Copy Markdown
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

ozyx commented Jul 1, 2017

Copy link
Copy Markdown
Contributor Author

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

paulvanbrenk commented Jul 5, 2017

Copy link
Copy Markdown
Contributor

I'll get a build out later today.

@ozyx

ozyx commented Jul 5, 2017

Copy link
Copy Markdown
Contributor Author

Great, thanks!

@paulvanbrenk

paulvanbrenk commented Jul 5, 2017

Copy link
Copy Markdown
Contributor

@paulvanbrenk paulvanbrenk mentioned this pull request Jul 10, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@paulvanbrenk paulvanbrenk paulvanbrenk approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ozyx @paulvanbrenk @msftclas