Fix git.parseTagData

[6543]

fix #14092

[6543]

@lunny thanks - good 👀!

EDIT: just saw the test error log too 🙃

[zeripath]

We need to do something slightly different here.

The issue is a that parseTagData is sometimes passed the terminal newline and sometimes not.

I think your code potentially is correct without the terminal newline but all of the rest of the calls need to be similarly limited.

Our unit tests should probably use []byte arrays too.

[6543]

@zeripath while test git migration from repo in reported issue ... out of 177 tags only 2 tags executed this function

So its not crear to me why this function exist ...

Is it some sort of workaround?

[zeripath]

The function exists because most tags in that repo are not annotated tags. parseTagData only applies to annotated tags.

[zeripath]

The TestAPIGitTags is also fundamentally incorrect because the message is always trimmed and terminated with a new line with -m

[khmarbaise]

Based on the fixture data which defined above (testData) this: if test.tag.Signature != nil is from my understanding always nil ? That would mean that the given if ... else is not necessary... Or do I misunderstand a thing here?
If it should not nil in some cases then some testData is missing?

[6543]

It could be simplified a bit yes

In sort, it's a custom assert.EqualValues(t, test.tag, tag) ... witch would be false in any case ... :/

[khmarbaise]

Ah sorry took a bit of time that you control the checking of the result of parseTagData based on the content of the fixture (testData)... now it makes sense to me... just missed that point.

[khmarbaise]

Here from my point of view the same thing because test.tag.Tagger would always not nil ?... WDYT?

[6543]

In short it 1. Check if value is expected ... 2. check if actual value is not nil (with assert test statement)
if expect & real is NOT nil, it checks the values
else (test already failed if expect != real, or nil is exptected - so...) test if result is nil