Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add XSD file for validation #2594

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

juherr
Copy link
Member

@juherr juherr commented Jun 8, 2021

  • Add test case(s)
  • Update CHANGES.txt

TODO:

  • upgrade schema version and remove deprecated
  • generate POJO from XSD.

@juherr juherr requested a review from krmahadevan as a code owner June 8, 2021 22:58
@juherr juherr mentioned this pull request Jun 8, 2021
@krmahadevan
Copy link
Member

@juherr - If you find some time please help let me know your thoughts on the two questions i posted. Post which I think we can get this PR merged.

@juherr
Copy link
Member Author

juherr commented Jun 10, 2021

@krmahadevan sure. But I don't see them. Did you finish the review?

<xsd:complexType>
<xsd:sequence>
<xsd:element ref="groups" minOccurs="0"/>
<xsd:choice minOccurs="0" maxOccurs="unbounded">
Copy link
Member

Choose a reason for hiding this comment

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

@juherr - Am assuming that we built this xsd on an as-is basis from the DTD. Should we move the below tags out from the minOccurs because I am not sure if anyone would give a value like this :

	<listeners>
		<listener class-name="foo"/>
	</listeners>
	<listeners>
		<listener class-name="bar"/>
	</listeners>
  • listeners
  • packages
  • method-selectors
  • suite-files

ideally speaking these elements are supposed to be present only once per <suite> tag.

WDYT ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally agree!

The dtd to xsd migration was not perfect and I missed things for sure

Copy link
Member

Choose a reason for hiding this comment

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

Oh you migrated this by hand is it ? I also tried some tool but it showed a lot of errors when i attempted to open up the generated xsd in visual studio code.

Are you going to be fixing this or we circle back to this later and for now go ahead with having an XSD as a starting point ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I used a tool for the generation but I had to fix it by hand.

I will adapt according to your remarks.

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, the default value for maxOccurs is already 1.

image

Copy link
Member

Choose a reason for hiding this comment

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

@juherr - If we put all the <xsd:element> within <xsd:choice> which has a min and max set, doesnt it mean that these tags can be repeated multiple number of times ? I dont see the same thing done for <xsd:element ref="groups">

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch.
I think groups should be added to the choice too.
I will try different options.

testng-core/src/main/resources/testng-1.0.xsd Outdated Show resolved Hide resolved
@krmahadevan
Copy link
Member

@krmahadevan sure. But I don't see them. Did you finish the review?

My bad. Forgot that. Now I finished it.

@juherr
Copy link
Member Author

juherr commented Jun 10, 2021

@krmahadevan Please check the description again before merging.
If you want to check XML samples, just add one suite file into the testng-core/src/test/resources/samples/xsd folder and my tests will check its validity.
If you think the XML is wrong and the test keep to be green, it will mean that the schema is not good enough =)

@juherr
Copy link
Member Author

juherr commented Jun 10, 2021

TODO: generate POJO from XSD.

@krmahadevan
Copy link
Member

@juherr - Can you please fix the merge conflict ? We can merge this after that.

@krmahadevan
Copy link
Member

ping @juherr

@juherr
Copy link
Member Author

juherr commented Jun 14, 2021

ping @juherr

I am doing my best to close it asap ;)

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.

None yet

2 participants