Add unit tests for File Metrics Collector

[andreyvelich]

/kind feature

We should add unit test for our File Metrics Collector to verify log parsing.
That will help to avoid issues similar to: #1754

/help
/good-first-issue

---

Love this feature? Give it a 👍 We prioritize the features with the most 👍

[google-oss-prow]

@andreyvelich:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/kind feature

We should add unit test for our File Metrics Collector to verify log parsing.
That will help to avoid issues similar to: #1754

/help
/good-first-issue

---

Love this feature? Give it a 👍 We prioritize the features with the most 👍

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tenzen-y]

/lifecycle frozen

[Electronic-Waste]

I want to have a try! It may help me better understand the logics of metrics collecting.

[tenzen-y]

I want to have a try! It may help me better understand the metrics collecting logics.

Sure, feel free to assign this yourself with a "/assign" comment.

[Electronic-Waste]

/assign

[Electronic-Waste]

@tenzen-y thank you!

[Electronic-Waste]

@tenzen-y I notice that you left a comment in the unit test file file-metricscollector_test.go:

TODO (tenzen-y): We should add tests for logs in TEXT format.

Does it mean that the additional test cases are supposed to be TEXT format? If so, can you provide an example of TEXT format log file for me? Thanks!

[tenzen-y]

@tenzen-y I notice that you left a comment in the unit test file file-metricscollector_test.go:

TODO (tenzen-y): We should add tests for logs in TEXT format.

Does it mean that the additional test cases are supposed to be TEXT format? If so, can you provide an example of TEXT format log file for me? Thanks!

Yes, that's right. We have UTs only for JSON form. So, we need to add UTs for TEXT form logs.
You probably could obtain samples once you run some example Experiments.

[kshitijdshah99]

@andreyvelich I would love to contribute to this issue just wanted to ask that I have to add unit tests for parseLogsInTextFormat and parseLogsInJsonFormat functions and modify test cases in this file, am I right??

[tenzen-y]

@andreyvelich I would love to contribute to this issue just wanted to ask that I have to add unit tests for parseLogsInTextFormat and parseLogsInJsonFormat functions and modify test cases in this file, am I right??

@kshitijdshah99 This is already taken by @Electronic-Waste. I'd be happy to be taken another issue by you.

[Electronic-Waste]

@tenzen-y Current error comparison in file-metricscollector_test.go is based on boolean rather than error content itself. To make test cases more detailed and accurate, we need to expose error and compare them. I would like to open another PR to fix this for this is a different feat compared with adding test cases for TEXT format log file.

[tenzen-y]

@tenzen-y Current error comparison in file-metricscollector_test.go is based on boolean rather than error content itself. To make test cases more detailed and accurate, we need to expose error and compare them. I would like to open another PR to fix this for this is a different feat compared with adding test cases for TEXT format log file.

SGTM

[tenzen-y]

/reopen

We need another followup PR.

[google-oss-prow]

@tenzen-y: Reopened this issue.

In response to this:

/reopen

We need another followup PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.