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
fix: xml validation (DEV-1360) #230
Conversation
|
||
def test_get(self) -> None: | ||
|
||
def test_create_lists(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is new
def test_excel_to_json_list(self) -> None: | ||
excel_to_json_lists.excel2lists(excelfolder='testdata/lists_multilingual', | ||
path_to_output_file='testdata/tmp/_lists-out.json') | ||
|
||
def test_excel_to_json_resources(self) -> None: | ||
excel2resources(excelfile='testdata/Resources.xlsx', | ||
path_to_output_file='testdata/tmp/_out_resources.json') | ||
|
||
def test_excel_to_json_properties(self) -> None: | ||
excel2properties(excelfile='testdata/Properties.xlsx', | ||
path_to_output_file='testdata/tmp/_out_properties.json') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved these three to the end, so that the file is ordered according the order in dsp_tools.py
excel2properties(excelfile='testdata/Properties.xlsx', | ||
path_to_output_file='testdata/tmp/_out_properties.json') | ||
def test_validate_xml_against_schema(self) -> None: | ||
self.assertTrue(validate_xml_against_schema(self.test_data_systematic_file)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
def test_create_project(self) -> None: | ||
result = create_project( | ||
input_file=self.test_project_systematic_file, | ||
server=self.server, | ||
user_mail=self.user, | ||
password="test", | ||
verbose=True, | ||
dump=False | ||
) | ||
self.assertTrue(result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved here from further down, so that the order is according to dsp_tools.py
|
||
|
||
def test_validate_project(self) -> None: | ||
self.assertTrue(validate_project(self.test_project_systematic_file)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, you would always test for the failure case too (even better, if you can test different failing scenarios).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
|
||
def test_validate_project(self) -> None: | ||
self.assertTrue(validate_project(self.test_project_systematic_file)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, you would always test for the failure case too (even better, if you can test different failing scenarios).
excel2properties(excelfile='testdata/Properties.xlsx', | ||
path_to_output_file='testdata/tmp/_out_properties.json') | ||
def test_validate_xml_against_schema(self) -> None: | ||
self.assertTrue(validate_xml_against_schema(self.test_data_systematic_file)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I agree with Balduin that also negative outputs / cases should be tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I can imagine there are more ways how the XML upload could fail. But it's a step in the right direction!
resolves DEV-1360