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 tests for documentation #106

Merged
merged 13 commits into from
Nov 21, 2017
Merged

Conversation

ogenstad
Copy link
Contributor

This is for the 2017 Hackathon
napalm-automation/hackathon-2017#10

This commit validates that each Ansible module has a DOCUMENTATION
and an EXAMPLES variable. It also asserts that those variables are
properly formatted YAML strings.

Copy link
Collaborator

@ktbyers ktbyers left a comment

Choose a reason for hiding this comment

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

I think this is a good idea.

The only thing I don't like is having two different set of tests defined in two different locations (one in .travis.yml and one in tox.ini).

I am sure I am going to get confused by that in the future.

What extra value is tox providing here (since we are PY27 only)?

tox.ini Outdated
deps =
-rrequirements.txt
pytest
ansible
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should create a requirements-dev.txt file and have pytest, ansible, tox in that as well as `-r requirements.txt.

@ogenstad
Copy link
Contributor Author

I agree with the comment about duplicate ways to test with tox. Mostly for now that was to give me an easy way to test. The only current value tox brings is that it tests that the package is actually installable and working. But I would say that tox either gets removed later, or the other tests gets moved in there too.

@dbarrosop
Copy link
Member

Yeah, this is cool. We should consolidate tests to use one tool though as kirk suggested

@ogenstad
Copy link
Contributor Author

I removed tox. Was just a bit lazy and installed a new environment instead. The above might need some other tests as there are some parts of the documentation missing in the modules now (such as defaults). Other than that I think this is mostly complete.

Please also comment on napalm-automation/napalm#535 :)

@ogenstad
Copy link
Contributor Author

I think I'm happy with this one now. There are other tests which can be added in the future and some extra controls.

Once this is merged I can add the git clone script in the NAPALM repo which uses the generated json files from here to build the documentation.

@ogenstad
Copy link
Contributor Author

Ok, so now this passes the test. I was only looking at the red paint which should actually be there but missed the:

The command "./test_changelog.sh" exited with 1.

Not sure that one always makes sense. Anyway, feel free to review. :)

@dbarrosop
Copy link
Member

awesome work!

@dbarrosop dbarrosop merged commit 1e36960 into napalm-automation:develop Nov 21, 2017
@ogenstad ogenstad deleted the docs branch November 21, 2017 08:06
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.

3 participants