-
Notifications
You must be signed in to change notification settings - Fork 6
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 chart tests #44
add chart tests #44
Conversation
@juttle/developers spamming everyone on this one since this does include everyone's work across the various components that make up juttle-engine. Right now just a quick set of tests to verify that barchart and timecharts work as expected within juttle-engine. The follow up PR for this will be to cover the other important charts we support and then ultimately start piecing together similar tests that start plugging in an adapter at a time using a docker container and verifying those produce the expected data through the whole pipeline and into a chart (even if to begin we simply dump to a text view to simply validate raw data). |
I think it's a good idea that we're doing integration tests, that said I think this particular suite of tests tends to focus on things that should be tested in juttle-viz (and are already test in juttle-viz). Running three tests the check for variations of options for barcharts really only tests stuff that juttle-viz should be testing. We could (more efficiently) test one a program that renders a barchart, timechart and table (with a couple mixed options) and we'd be covering a larger area of integration pieces:
My beliefs in summation- Only use browser based, selenium tests when you REALLY have to and sparingly as possible. Trust that the various repos are properly testing their isolated responsibilities. |
Also the adapter integration tests can be done without selenium and by calling the api directly. |
We have established a goal of having end-to-end test coverage, so the conversation here is not about whether to have tests that read data in (via adapters, soon enough) and validate that we can see a chart - we will absolutely do so - only about which specific test cases to cover in e2e scenario vs at a lower level of integration tests. @mnibecker can you point out which tests in this PR are unnecessary because they are already covered in juttle-viz or client library? |
w.r.t to testing juttle-client-library: These all test whether it can run a program against a live juttle-service and properly instantiate the appropriate juttle-view and pass it the correct option. We definitely need to have an integration test that this works, I suggested doing one with multiple charts. (we already unit tests the layout mechanism, client api's against a mock service, and job-manager api in jcl). w.r.t to tests in juttle-viz: @go-oleg correct me if I'm wrong, but I don't think we test as a whole the charts spit out certain dom based on a given set of parameters. I do know we test individual generators (bars included). Not questioning whether these tests should exist, I'm just questioning whether the level of granularity might be more appropriate for juttle-viz. |
Right, we don't inspect the DOM and verify that its correct, thats something that was done in the "system tests" in Jut 1.0. |
fixes #43
basic set of chart tests for barchart, timechart and event overlays on timecharts.