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

SUMO-111064 Add unit test for events plugin #63

Merged
merged 11 commits into from Jul 11, 2019
Merged

SUMO-111064 Add unit test for events plugin #63

merged 11 commits into from Jul 11, 2019

Conversation

yuting-liu
Copy link
Contributor

@yuting-liu yuting-liu commented Jul 9, 2019 •
edited

Add unit tests for events plugin:

  • pull_initial_resource_version
  • initialize_resource_version
  • watch events: by default only with type ADDED and MODIFIED. Customers can also decide type_selector by configuration to decide whether DELETED is also selected
  • watch other resources

samjsong
samjsong reviewed Jul 9, 2019
View reviewed changes
Copy link
Contributor

@samjsong samjsong left a comment

Choose a reason for hiding this comment

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

Overall LGTM, thanks Yuting! Left a few minor comments

fluent-plugin-events/test/plugin/test_in_events.rb Outdated Show resolved Hide resolved
fluent-plugin-events/test/plugin/test_in_events.rb Outdated Show resolved Hide resolved
maimaisie
maimaisie reviewed Jul 9, 2019
View reviewed changes
Copy link
Contributor

@maimaisie maimaisie left a comment

Choose a reason for hiding this comment

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

These are very good test cases for happy paths. Can you add some test cases for things like an invalid type_selector array, or an ERROR type event that says the supplied RV is expired (and making sure that event is not ingested)? It is always important to test if the component has good error handling.

fluent-plugin-events/test/helper.rb Outdated Show resolved Hide resolved
fluent-plugin-events/test/plugin/test_in_events.rb Outdated Show resolved Hide resolved
samjsong
samjsong previously approved these changes Jul 10, 2019
View reviewed changes
samjsong
samjsong reviewed Jul 11, 2019
View reviewed changes
Copy link
Contributor

@samjsong samjsong left a comment

Choose a reason for hiding this comment

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

Just noticed we're doing assert_equal actual, expected, but I believe the ordering should be assert_equal expected, actual or the test failure message will display the message incorrectly

@yuting-liu
Copy link
Contributor Author

@samjsong you are right. I think in some of our other test files (test_cache_strategy.rb and test_reader.rb), assert doesn't in the right order of assert expected, actual way. Maybe we should fix them all?

@samjsong
Copy link
Contributor

Good catch, I think we can fix those in another PR :)

@yuting-liu yuting-liu merged commit 56b1dc2 into master Jul 11, 2019
@yuting-liu yuting-liu deleted the yuting-events-ut branch July 11, 2019 20:29
psaia pushed a commit to psaia/sumologic-kubernetes-collection that referenced this pull request May 25, 2021
@mlclmj
Add FTP collectors (SumoLogic#63)
d8a1b07 
* Add ftp collectors

* Update collectors.tf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maimaisie maimaisie maimaisie approved these changes

@samjsong samjsong samjsong approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@yuting-liu @samjsong @maimaisie