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

Cleaning up the DTD to be stricter with validation #46

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

krmahadevan
Copy link
Member

Copy link
Member

@juherr juherr left a comment

Choose a reason for hiding this comment

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

The dtd is an artefact. You can modify it once published. We need to create a new minor version and treat it in the parser

@krmahadevan
Copy link
Member Author

The dtd is an artefact. You can modify it once published. We need to create a new minor version and treat it in the parser

You mean to say that we need to create a new file called testng-1.1.dtd and make the changes into that ?

if so, then it will be used ONLY when a user updates

  • Old value: <!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd"> to
  • New value: <!DOCTYPE suite SYSTEM "https://testng.org/testng-1.1.dtd">

in each of their suite files, which I doubt if anyone is ever going to be making.

So what are we going to be achieving by publishing a new version?

I am curious to u'stand this aspect.

@krmahadevan
Copy link
Member Author

@juherr - I have copied contents of testng-1.0.dtd into testng-1.1.dtd and made the required changes. Please check

@juherr
Copy link
Member

juherr commented Jun 7, 2023

Remember that we have a warning when no dtd is set.

@juherr
Copy link
Member

juherr commented Jun 7, 2023

Do you confirm that the new dtd complains when used on a file with children under suite-file?

@krmahadevan
Copy link
Member Author

@juherr - I went through the TestNG codebase and here's what I found

  • We try to load the dtd that is available in src/main/resources
  • If that fails, then we fall back to loading the same dtd from testng.org

So I think that if we check-in the new dtd into testng codebase (which I have done in the other PR) then we should be good.

@juherr juherr merged commit d1b6e8f into testng-team:master Jun 7, 2023
@krmahadevan
Copy link
Member Author

Do you confirm that the new dtd complains when used on a file with children under suite-file?

I tested this locally and this is what I see in IntelliJ

image

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.

Parameters of nested test suites are overriden
2 participants