-
Notifications
You must be signed in to change notification settings - Fork 8
Make use of the general config section and split the parsers + test coverage #10
Conversation
Pre-Commit Test failed! Click here
|
Pytest Test failed! Click here
|
4608f15
to
6d52960
Compare
Pytest Test failed! Click here
|
Pre-Commit Test failed! Click here
|
Pytest Test failed! Click here
|
Pre-Commit Test failed! Click here
|
Unfortunately sesheta will keep complaining in this repo for a while. Locally the pre-commit passes. Ready to be reviewed. |
/retest |
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.
Ran pipenv install --dev && pipenv run pytest .
successfully, lgtm -- I'd suggest adding to the readme on how to run the tests!
🚢
@tumido , aicoe-ci did not receive any webhooks, is the app disabled for this repo ? |
@harshad16 no idea. Was it even set up? 🤷 When I push to the PR it gets retested but I have no idea how it works behind the scenes. You should have admin access to this repo, can you please check it is set up properly? |
@harshad16 I'll leave this PR open for today, feel free to use if for testing the testing 👍 |
thanks, for keeping this open. i will ping you as soon as i figure this out. |
/retest |
Pre-Commit Test failed! Click here
|
yes, it is correctly set up. |
/retest |
@harshad16 I'm merging this PR now. You can continue your triage on #13 (I'll rebase it on top of this one + master) |
Pre-Commit Test failed! Click here
|
Thnaks @tumido , i think i have updated aicoe-ci. |
Related Issues and Dependencies
#1
This introduces a breaking change
This Pull Request implements
Instead of one single parser, split it into 2 more distinct parsers:
source_*
anddestination_*
s )solgate
, that is now utilized fortimedelta
(will be used by the notification service as well)Adding test coverage for all the config parsings.