Skip to content
This repository has been archived by the owner on Nov 17, 2022. It is now read-only.

Check is_iati_xml() during full_validation() #239

Merged
merged 6 commits into from
Nov 16, 2017
Merged

Conversation

hayfield
Copy link
Contributor

As part of full validation, it should be checked that data is IATI XML. This PR adds this.

Certain tests have been refactored so that full_validation() is tested within the TestValidatorFullValidation class rather than in other areas.

Fixes #238

Previously, the checks for Rulesets being in full validation were checked at the Ruleset-conformance level rather than the full_validation level.
This new organisation better groups associated tests.
Checking that something is valid XML is performed before the Codelist checks.
As such, it makes some amount of sense that the tests are in this order.
This step was missing.
To add it in, an error needs raising and handling for when a non-Dataset is provided.
@hayfield hayfield added tests Relating to tests and testing. validation Changes to validation functionality. labels Nov 15, 2017
@hayfield hayfield requested a review from a team November 15, 2017 09:34
@hayfield hayfield added the complete A PR that is in a state that is ready for review. label Nov 15, 2017
Copy link
Contributor

@allthatilk allthatilk left a comment

Choose a reason for hiding this comment

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

Only a lil' change =)

@@ -445,6 +446,8 @@ def _check_is_iati_xml(dataset, schema):
for log_entry in doc_invalid.error_log: # pylint: disable=no-member
error = _create_error_for_lxml_log_entry(log_entry)
error_log.add(error)
except AttributeError:
raise TypeError('The provided argument that should have been a Dataset was actually a {0}'.format(type(dataset)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe better to word:
('Unexpected argument: {0} is not an iati.data.Dataset'.format(type(dataset)))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per suggestion in review of #239
@hayfield hayfield dismissed allthatilk’s stale review November 16, 2017 10:50

Requested change made :)

@hayfield hayfield requested a review from a team November 16, 2017 10:51
Copy link
Contributor

@allthatilk allthatilk left a comment

Choose a reason for hiding this comment

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

:shipit:

@hayfield hayfield merged commit 10f18da into dev Nov 16, 2017
@hayfield hayfield deleted the full-valid-iati-xml branch November 16, 2017 15:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
complete A PR that is in a state that is ready for review. tests Relating to tests and testing. validation Changes to validation functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants