Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial submittion of a reusable hysteresis function. #222
Initial submittion of a reusable hysteresis function. #222
Changes from 4 commits
6aa68b3
eafebf1
adde2e0
7e63aaf
601aa28
c87075b
bc3af91
9cc968f
b70d159
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take this out and put in into a comment in the PR or add to a new Tests directory off of the root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've moved these, but put the Tests directory under
.github
. Please move it up to the root (same level as Core and Community).I'm rethinking this directory structure though. I think it would be better to put the tests in packages, like
/Tests/automation/lib/python/tests/core/utils.py
. The ones for hysteresis would go in there and we can add in others for the rest of utils. Then we can use a script to load the module and execute the tests. Still just drawing in the sand though!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense for Communty to put the tests with the modules? For example:
Modules:
Community/Gatekeeper/automation/lib/python/community/gatekeeper/gatekeeper.py
Scripts:
Community/Gatekeeper/automation/jsr223/python/community/gatekeeper/gatekeeper_script.py
Tests:
Community/Gatekeeper/automation/test/python/gatekeeper.py
That way the tests and the code would live in the same root folder which might make it easier for contributors to manage.
For tests on the Helper Libraries themselves, the second "tests" feels redundant. Is there a standard for unit tests for Python? Maybe we should follow that (and for the other languages as well.
Anyway, obviously VSCode let me down when I created the folder. It'll be fixed shortly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of any standards for tests and I've seen it done many different ways. I think it would be best to keep the tests separate, since they don't really have a place in a normal installation. To do this, they'd need to go into a separate directory off of the root of the repo. Under that directory, they'd have the normal directory structure so that they can be copied over an existing installation, or patched in with a symlink.
There's a lot to sort out for testing and this is just a rough cut to get them stashed somewhere. It will be nice to have some tests in the repo to have some experience using them to help determine the best location and workflow for them. There were some other tests built up for core.metadata and core.date, and I have pages of regression tests that I use after major changes in OHC. We'll get there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, well the commit below moved the tests to the folder you suggested above. This issue should be addressed.