Change in commit message leads to multiple unit tests failing

[Nati Elgavi]

Hi,

I am new to the concept of unit tests so it might be a noob's question. 

I have made a change in the commit message. 

I didn't modify the git-tfs-id line, but I have added a line before it that provides a link to the TFS changeset through the web access interface on our server. 

For example: 

original-commit-message....

 

tfs-changeset: http://team:8080/tfs/DefaultCollection/_versionControl/changeset/123 

git-tfs-id: [http://team:8080/]$/sandbox;C123

I added this line in the BuildCommitMessage function (see 3DSystemsAdaptations branch at https://github.com/NatiElgavi/git-tfs.git ).

I also modified the only two tests that use the AssertCommitMessage function to include the additional line.

Still, as you can see in the attached log, several tests failed.

When I tried to look into this, I saw that the tests include a hardcoded expected sha.

How is this supposed to work?

Is it expected that changing the commit message will affect the sha of the commit?

I'm not sure if the test indicate that I did something wrong or that I should fix the test.

Thanks,

Nati.

[Philippe Miossec]

 

How is this supposed to work?

Is it expected that changing the commit message will affect the sha of the commit?

In git, every piece of information that changes (files, commit message, date, author,...) changes the commit sha. Because the sha is processed with all the data in input. 

I'm not sure if the test indicate that I did something wrong or that I should fix the test.

So, yes, you should adapt the test (that are more integration tests than unit test ;-) ) 

[Nati Elgavi]

Thanks for the explanation. 

I'll review the test results again and hopefully other than mismatch on the sha (due to the changed commit message) I'll be good to go.

If this added line if of interest to anyone, I will fix the tests and send a pull request. 

I have never done that before either, so any hint you can give me on the type of changes that are usually accepted, I'll be grateful-er :)

Nati.

[Philippe Miossec]

Thanks for the explanation. 

I'll review the test results again and hopefully other than mismatch on the sha (due to the changed commit message) I'll be good to go.

👍

If this added line if of interest to anyone, I will fix the tests and send a pull request. 

I have never done that before either, so any hint you can give me on the type of changes that are usually accepted, I'll be grateful-er :)

There is no other rule for a PR to be accepted than if this could be useful for others and that the quality is good enough. 

My advices:

* Create the PR as soon as you can that way we could follow your developments and give you some advices, ask you to change some things,... It will be less painful to have these feedbacks earlier. If your development is not finished, just add '[WIP]' at the start of the title of your PR. 

* There is already  '--export' and '--export-work-item-mapping' option to had the workitem ids in the commit message (and other metadata). I think your development should play nicely with these existing options.

Add an option '--workitems-with-url' (naming not definitive, if you find something better) 

Thanks for the contribution!