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.

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

Add tests for the tsdb cli tool #5864

Closed
krasi-georgiev opened this issue May 30, 2019 · 9 comments · May be fixed by prometheus-junkyard/tsdb#673
Closed

Add tests for the tsdb cli tool #5864

krasi-georgiev opened this issue May 30, 2019 · 9 comments · May be fixed by prometheus-junkyard/tsdb#673

Comments

@krasi-georgiev
Copy link
Contributor

can start with simple test for the ls, analyze and dump commands.

  1. create a block with createBlock()
  2. run each command
  3. compare expected vs actual output.
@obitech
Copy link

obitech commented Jul 19, 2019

Hey @krasi-georgiev, I would like to work on this feature. Before I stumpled upon this issue I was actually thinking about writing my own little CLI tool that will send a request to the Prometheus API, create a snapshot and then send it to S3 or something. :)

This would be my first contribution to the project and I couldn't find any documentation on how to use the CLI tool for snapshots but I assume with createBlock() you're referring to https://github.com/prometheus/tsdb/blob/31f7990d1d49f1637f6021c99647f22d56ed3f86/block_test.go#L210 ? I've spent some time going through the code but I have a couple of general questions, would you have 10 or 15 minutes for a quick chat on IRC or Zoom sometime?

@krasi-georgiev
Copy link
Contributor Author

krasi-georgiev commented Jul 19, 2019

Prometheus itself has an api to create snapshots:
https://www.robustperception.io/taking-snapshots-of-prometheus-data

@krasi-georgiev
Copy link
Contributor Author

sorry I didn't read your comment properly. Yes feel free to ping me on #prometheus-dev irc channel

@krasi-georgiev
Copy link
Contributor Author

I think you can use https://golang.org/pkg/testing/#hdr-Examples

@obitech
Copy link

obitech commented Jul 23, 2019

Hey @krasi-georgiev I've tried setting up Example-Tests, however I'm running into some issues. Summary: it's a bit gnarly and feels kind of wrong. Check my code here.

  • We can't use *testing.T in those tests since those Example test function take no arguments
  • go vet is complaining because the test function is not referring to any actual examples in the docs.
  • We can't use variables when checking the output. This makes testing for BLOCK ULID rather difficult, if not impossible.

I'm not too familiar with the inner workings of those Example Tests but it feels like abusing this functionality for our use case 😬

I'm gonna try to capture the output in a different way, but the whole table writer formatting / tab-style formatting makes it extra-hard to test for the whole string. I'm gonna play around a bit and see if I can get it right. Although I might be missing something, please let me know if you have any feedback/ideas.

@krasi-georgiev
Copy link
Contributor Author

Another way I can think of is passing a logger to these funcs so when you test them you can pass some mock logger that can than be used to test the output.

@obitech
Copy link

obitech commented Jul 25, 2019

That's a good idea. I'll be busy the next couple days but I hope to make some progress next week.

@obitech
Copy link

obitech commented Aug 4, 2019

So I think I've found a way that is not too bad:

  • printBlocks in main.go now takes an w io.Writer (which is by default os.Stdtout in main()) which makes it easier to test.
  • In main_test.go we can then pass a bytes.Buffer as w where we can capture the output.

One very annoying thing is that os.Stdout will automatically expand tabs according to the length of meta.ULID which is very annoying to test. For this I have stripped all white space characters of the expected and actual output, to just compare the raw data that is emitted.

I have a created a WIP PR (#673) so you can see all my changes, maybe we can continue the discussion over there on how to proceed with this issue?

@krasi-georgiev
Copy link
Contributor Author

not relevant as the tsdb tool has been merged with the promtool

@prometheus prometheus locked as resolved and limited conversation to collaborators Nov 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants