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

Figure out way to allow for definition tests to be run against definitions in this repository #42

Open
ppeble opened this issue May 3, 2017 · 3 comments

Comments

@ppeble
Copy link
Member

ppeble commented May 3, 2017

We have a problem. Currently it is possible for someone to submit definition changes, not update the related tests, and everything will return 'green'. The make validate only ensures that the syntax is right, not the meaning behind the syntax. This can lead to situations that require PRs like this: #41

Through no fault of the submitter I only found the problem when I was about to release the next version of the ruby gem.

I think we could do something like this:

  1. Complete Change 'tests' in definitions to be more generic #7 (DONE)
  2. Complete Change 'methods' in definitions to be more generic #24 (DONE!)
  3. Add a way to run the updated definitions against all consuming apps. This should probably be a separate script (shell or ruby, either way) that will clone the consuming apps (right now just ruby), modify it so it points to the modified definitions, re-generate, and then run the make test command for the app. This would prove that the new defs were behaving as expected and did not lead to any parsing errors.

Only after we have done this will this repo be truly standalone. The current straddling that we are doing is not great for submitters and, over the long run, will cost us contributors.

@ppeble
Copy link
Member Author

ppeble commented Mar 13, 2019

Note for the future: I have documented how you would update the ruby repo to pull definitions from a branch here: https://github.com/holidays/holidays/blob/master/doc/CONTRIBUTING.md#testing-out-your-definitions-locally

@hlascelles
Copy link
Contributor

I'm still very keen for definitions to be "self testable", even if just a bit.

Case in point, the new ke.yml file has invalid indentations which make it unparsable.

Would you be open to a PR to add, say, GitHub actions to at least check the files are well formed on any new PRs submitted to this repo? And probably, move the ruby code from the holidays gem to this one to check the "test" blocks are valid?

@ppeble
Copy link
Member Author

ppeble commented Dec 26, 2023

This is still a very real problem. I feel as if every release ends up hitting this. The 'validate' passes in this repo and then fails in the ruby repo. We need to add in the ability to pull the ruby repo, update to this branch, and run it before accepting it. This is not hard! We should be able to do it without too much effort. I think I'll concentrate whatever spare cycles I have to this specifically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants