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.

Sign up for GitHub

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

Jump to bottom

test_encode: rewrite test cases in criterion #2618

Merged
merged 1 commit into from
Mar 13, 2019
Merged

test_encode: rewrite test cases in criterion #2618

merged 1 commit into from
Mar 13, 2019

Conversation

mehul-m-prajapati
Copy link
Contributor

fixes #2580

@kira-syslogng
Copy link
Contributor

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
do nothing -> CI won't start)

1 similar comment
@kira-syslogng
Copy link
Contributor

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
do nothing -> CI won't start)

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 9, 2019

@kira-syslogng ok to test

@mehul-m-prajapati mehul-m-prajapati force-pushed the test-encode-criterion branch 2 times, most recently from ae13b9e to bd516f9 Compare March 9, 2019 20:26
@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@furiel
Copy link
Collaborator

furiel commented Mar 11, 2019

@mehul-m-prajapati can you resolve the conflict?

@mehul-m-prajapati
Copy link
Contributor Author

Hi @furiel ,

Ok. I will resolve it.

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build SUCCESS

Kokan
Kokan previously approved these changes Mar 12, 2019
View reviewed changes
furiel
furiel suggested changes Mar 12, 2019
View reviewed changes
Copy link
Collaborator

@furiel furiel left a comment

Choose a reason for hiding this comment

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

Similarly to #2584 the testing happens in the complex assert functions, that are fed from a set of inputs. Please turn test_encode_simple_strings, test_encode_strings_that_need_quotation and test_encode_strings_with_forbidden_chars into parametrized test.

@kira-syslogng
Copy link
Contributor

Build FAILURE

@mehul-m-prajapati
test_encode: rewrite test cases in criterion
76948b3 
Signed-off-by: Mehul Prajapati <mehul.encs@gmail.com>
@MrAnno
Copy link
Collaborator

MrAnno commented Mar 12, 2019

@mehul-m-prajapati We have some internal issues with @kira-syslogng, we'll fix that ASAP.
Your PR is fine, thanks for the hard work!

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@folti
Copy link
Contributor

folti commented Mar 12, 2019

@kira-syslogng retest this please

@kira-syslogng
Copy link
Contributor

Build FAILURE

@folti
Copy link
Contributor

folti commented Mar 12, 2019

@kira-syslogng retest this please

@kira-syslogng
Copy link
Contributor

Build SUCCESS

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 12, 2019

@folti 🎉

@MrAnno MrAnno merged commit 2713595 into syslog-ng:master Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrAnno MrAnno MrAnno approved these changes

@furiel furiel furiel approved these changes

@Kokan Kokan Kokan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

convert test_encode to criterion
6 participants
@mehul-m-prajapati @kira-syslogng @MrAnno @furiel @folti @Kokan