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

Update PDK to 1.18.1 and re-work tests #141

Merged
merged 6 commits into from
Oct 23, 2020

Conversation

nward
Copy link
Collaborator

@nward nward commented Oct 23, 2020

Hi there!

I'm reworking a puppet environment and as part of that we're throwing out our internal freeradius module and have elected to use this one. There are some changes required to get it to where we want it, so I'm contributing these changes back. This is the first of these.

Here I am updating PDK to 1.18.1. I had intended to test before and after, but found that the tests seemed to be broken and unmaintained. I've re-worked all the tests away from the generated tests so they can be maintained going forward. My focus has been to retain the original test cases (i.e., what was being tested) so have only added a few additional test cases and contexts. It is my intent to add more test cases and contexts however, if this PR is accepted, as part of other PRs I intend to send.

@djjudas21
Copy link
Owner

Awesome work, thanks! It's great when someone feeds back improvements.

Don't worry about doing destructive work on the tests or preserving anything - I just ran a script which generated a skeleton set of tests, which never worked, and I never got further than that with testing.

Unfortunately I changed jobs after that and I no longer use this module myself, so maintenance has really been limited to curating PRs that people such as yourself have kindly sent 🙂

Your PR looks great but I no longer have any means of testing it in a real environment, so I have eyeballed it and I'm happy to accept the PR. Look forward to seeing what other improvements are in the pipeline for your use case.

@djjudas21 djjudas21 merged commit e1dec8c into djjudas21:master Oct 23, 2020
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.

2 participants