Skip to content

Conversation

@VadimKulagin
Copy link

Original issue: #1
I made the necessary changes and updated the tests.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 94.444%

Totals Coverage Status
Change from base Build 5: 0.04%
Covered Lines: 136
Relevant Lines: 144

💛 - Coveralls

@danilobuerger
Copy link
Owner

Hi @VadimKulagin sorry for taking such a long time to get to this. Could you please explain shortly the changes you made to the tests and the intention behind it?

@VadimKulagin
Copy link
Author

Hi @danilobuerger.

Initially, I did not want to change tests so much, but I had to do it because their initial implementation did not allow me to perform a value check, which can have two views in XML at once.

For example, the value of nil in XML can be represented simply as <Value xsi:nil="true"></Value>, or maybe <ValueXmlWrapper xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><Value xsi:nil="true"></Value></ValueXmlWrapper>.

The initial implementation of the tests did not allow testing the second option, because each test ​​had a hard format:

type nullTest struct {
	new       func() nullTestValue
	jsonValue nullTestValue
	json      string
	textValue nullTestValue
	text      string
	xmlValue  nullTestValue
	xml       string
}

Therefore, I had to make a separate test for each type of values, in which I can specify the initial and expected value.

type nullTest struct {
	Type        testType
	New         func() interface{} // Generator of *nullTestValue (and their wrappers)
	Unmarshaled interface{}
	Marshaled   string
}

And now I can specify different input and output data for the test.

	tests := []nullTest{
		{testTypeXml, int64NullValue, int64NullValue(), `<Int64 xsi:nil="true"></Int64>`},
		{
			testTypeXml,
			newInt64NullValueXmlWrapper,
			newInt64NullValueXmlWrapper(),
			`<nullTestValueXmlWrapper xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><Value xsi:nil="true"></Value></nullTestValueXmlWrapper>`,
		},
	}

Thus, it became possible to test the conversion to XML and from XML with and without namespace.

@VadimKulagin
Copy link
Author

Hello! What do you think about this pull request?

If my changes look too big, then I can do another pull request, in which there will be a separate test only for my case with XML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants