Skip to content

Conversation

@ronensc
Copy link
Contributor

@ronensc ronensc commented Aug 3, 2022

Unmarshaling non-config related json still use the non-strict version.

This is a follow-up on #271
Fix #274

@ronensc ronensc requested review from KalmanMeth, eranra and jotak August 3, 2022 08:31
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2022

Codecov Report

Merging #279 (be64c8d) into main (d7656a2) will increase coverage by 0.03%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
+ Coverage   66.85%   66.89%   +0.03%     
==========================================
  Files          73       73              
  Lines        4152     4157       +5     
==========================================
+ Hits         2776     2781       +5     
  Misses       1197     1197              
  Partials      179      179              
Flag Coverage Δ
unittests 66.89% <92.30%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/confgen/confgen.go 49.20% <50.00%> (ø)
pkg/confgen/encode.go 50.00% <100.00%> (ø)
pkg/confgen/extract.go 50.00% <100.00%> (ø)
pkg/confgen/transform.go 46.66% <100.00%> (ø)
pkg/confgen/visualization.go 50.00% <100.00%> (ø)
pkg/config/config.go 63.63% <100.00%> (+8.08%) ⬆️
pkg/test/utils.go 65.00% <100.00%> (+0.29%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

}

func DeserializeJSONToMap(t *testing.T, in string) config.GenericMap {
t.Helper()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the practical result of declaring this function as a Helper function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KalmanMeth
By marking a function as helper, when a test fails in the helper function, the line number reported will be of the caller function rather than inside the test helper function.

Consider running the following test:

package b

import (
	"testing"
)

func Test(t *testing.T) {
	helperMarked(t, "hello", "hi") // line 8
	helperUnmarked(t, "hello", "hi")
}

func helperMarked(t *testing.T, expected, actual string) {
	t.Helper()
	if expected != actual {
		t.Errorf("expected (%v) != actual(%v)", expected, actual)
	}
}
func helperUnmarked(t *testing.T, expected, actual string) {
	if expected != actual {
		t.Errorf("expected (%v) != actual(%v)", expected, actual) // line 20
	}
}

The result is:

=== RUN   Test
    b_test.go:8: expected (hello) != actual(hi)
    b_test.go:20: expected (hello) != actual(hi)
--- FAIL: Test (0.00s)

FAIL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, I didn't know that

Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ronensc ronensc merged commit fb310b7 into netobserv:main Aug 3, 2022
@ronensc ronensc deleted the json-unmarshal-strict branch August 3, 2022 12:54
@jotak jotak added the breaking-change This pull request has breaking changes. They should be described in PR description. label Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This pull request has breaking changes. They should be described in PR description.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash FLP on an incorrect config file

4 participants