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

Sitemap Check / Validation #10

Merged
merged 11 commits into from
Oct 11, 2016
Merged

Sitemap Check / Validation #10

merged 11 commits into from
Oct 11, 2016

Conversation

Johanndutoit
Copy link
Contributor

Does a check for the sitemap as well as validation if present. We're not too worried about sitemaps but if one is defined it should be in the right format, and if none are present the url /sitemap.xml should not return a catchall page that renders the homepage or something.

Consider this the first version, it currently checks:

  • If the sitemap.xml defined is text/xml or application/text which should stop any HTML pages through
  • If the sitemap.xml defined is even parseable as XML ?
  • If the sitemap.xml has a correctly defined prolog and all the required sections as part of the checks
  • If the sitemap.xml conforms to the spec and does not have any unknown properties
  • If the sitemap.xml conforms and has all the required properties in the nodes

Going to see about adding actual value validation for each of the properties as they all have their own specs for how the data should be presented

@Johanndutoit
Copy link
Contributor Author

See node v0.12 requires some work as xmltojs does not work in that version, will try to find a solution for that one. Rest of the tests run fine from > v0.12

@Johanndutoit
Copy link
Contributor Author

Upped the version required to v0.12, it's not worth supporting v0.12 as it's dropping of end of the year anyway

Copy link
Contributor

@BarryBotha BarryBotha left a comment

Choose a reason for hiding this comment

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

Just some nitpicking grammar issues


];

// get the robots from site
Copy link
Contributor

Choose a reason for hiding this comment

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

Referencing robots and not sitemap

if(err) {

// output to log
payload.error('Problem checking the robots txt', err);
Copy link
Contributor

Choose a reason for hiding this comment

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

Referencing robots and not sitemap

// add the occurence
payload.addRule(rulePrologMeta, {

message: 'Sitemap prolog have $ as a closing tag',
Copy link
Contributor

@BarryBotha BarryBotha Oct 10, 2016

Choose a reason for hiding this comment

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

have must be has

// add the rule
payload.addRule(ruleSchemaMeta, {

message: 'The root node of the site ($) should contain only $ nodes',
Copy link
Contributor

Choose a reason for hiding this comment

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

sitemap

describe('sitemap', function() {

// add the test for the robot
it('Should not return a error if the /sitemap.xml url returned anything else than a 200', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

an error


// check for a error
if(err)
assert.fail('Was not expecting a error');
Copy link
Contributor

Choose a reason for hiding this comment

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

an error


// check if we found it
if(!rule)
assert.fail('Was expecting a rule to return');
Copy link
Contributor

Choose a reason for hiding this comment

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

an error

});

// add the test for the robot
it('Should not return a error if the sitemap returned as application/xml', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

an error


// check for a error
if(err)
assert.fail('Was not expecting a error');
Copy link
Contributor

Choose a reason for hiding this comment

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

an error

});

// add the test for the robot
it('Should not return a error if the sitemap returned as text/xml', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

and so for all of them...

@Johanndutoit
Copy link
Contributor Author

Johanndutoit commented Oct 10, 2016

@BarryBotha ^ grammar fixes as requested (oops)

@Johanndutoit
Copy link
Contributor Author

Merging :)

@Johanndutoit Johanndutoit merged commit 7efefc2 into develop Oct 11, 2016
@Johanndutoit Johanndutoit deleted the sitemap branch October 11, 2016 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants