Add a test for PR # 685

[purpleidea]

#685 (comment)

Would be great to add a simple test for this commit and also to double check that the test fails if that commit is reverted.

Commit: bf7e454

Thanks to @frebib for the nice patch!

[rafiramadhana]

@purpleidea
Hi.
I can help with this, I am aware of what should be tested in regards to commit bf7e454.
However, I still don't know what does the term kind means inside the StructKindToFieldNameTypeMap(kind string) (map[string]*types.Type, error).
Can you help to pinpoint me some examples of how this struct kind looks? Maybe something like testdata? Thank you.

[rafiramadhana]

@purpleidea
Hi again.
After playing around with the StructKindToFieldNameTypeMap method using tests under the ./engine folder, it looks like to test the StructKindToFieldNameTypeMap, we should have a registered resource in the registeredResources map.
Do we already have the convention to conveniently register a resource when doing tests?
However, I found this TestRes struct. But this struct can't be imported in ./engine/util as it will result to an import cycle.
Wdyt? Thanks.

[purpleidea]

@JefMasereel can probably point to the issue he found that this patch fixed... After our memory is refreshed, we can use TestRes (and import it elsewhere) or do something different. Does this help?

[purpleidea]

Original issue is here actually: #684

[rafiramadhana]

@purpleidea
Ok, I will take a look at #684 first.

Anyway, by "...our memory is refreshed", do you mean clearing the registeredResources?

I think we can create a new file of ./engine/util/test.go, then copy the implementation of TestRes (and its related dependencies) in ./engine/resource/test.go to that new file. Although it looks like a duplication, I think it is tolearble, considering that:

• util and resource are different packages
• TestRes is less likely to change (I see that the last change in TestRes is around 4 yrs ago)

Wdyt? Thanks.

[rafiramadhana]

@purpleidea
Oh ya, what does the term kind in this project means ya? I am quite new to this domain.
Could you provide me some articles to read?
Thank you.

[purpleidea]

@neverbeenthisweeb Sure!

Kind has a few uses, but in terms of the resources in mgmt, kind is file, svc, pkg, virt, etc... In other words the "type" of resource, although we don't use the word type here for various reasons.

[purpleidea]

Oh, and for articles: https://github.com/purpleidea/mgmt/blob/master/docs/on-the-web.md

You might want to read the oldest one by me, and maybe the first language one. There are some videos of conference talks you can watch if you're interested. Newer ones are more current and more useful.

[JefMasereel]

The Hetzner resource that I'm working on was failing type unification due to a circular type structure in one of the hcloud-go values that I store in a private resource field. @frebib added this patch to skip type checks for unexported fields, which solved the issues I was having. purpleidea has listed all the relevant issues by now, not sure if I can help beyond that at the moment.

[rafiramadhana]

@purpleidea @JefMasereel
Got it, I will try to grasp those information first.

Thanks for the help guys.

[ofekatr]

I see that the implementation was revised in aff6331:

mgmt/engine/util/util.go

Lines 279 to 283 in aff6331

	// Skip unexported fields. These should never be mapped golang<->mcl.
	//if field.PkgPath != "" { // pre-golang 1.17
	if !field.IsExported() {
	continue
	}

Is this issue still relevant?
I've opened a PR, a review would be appreciated

#709

cc @JefMasereel @purpleidea

[purpleidea]

@ofekatr Thank you for the PR, I sent an initial review.
If you're looking for some mgmt patches to write, ping me and happy to guide you. Cheers!