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

Rework the logic and the tests of the graph visualization type checkboxes. #2193

Merged
merged 2 commits into from
Aug 25, 2017

Conversation

QuentinBrosse
Copy link
Contributor

Description

  • In addition to the active class on the visualization type checkboxes. I have added the checked state of the input. (Also, this modification fix the selenium test test_synchronizes_fields_on_graph_tab.)
  • Transform the failing selenium test test_keeps_svg_visualisation_after_page_refresh into a test_keeps_visualisation_type_after_page_refresh in order to make it more useful (in my opinion).

Motivation and Context

Before beginning the rework / recode of the visualizer (#2172). It it was important to fix all the tests of the current one.
Before this PR, it was not the case.

Have you tested this? If so, how?

I ran my jobs and the unit tests with this code and it works for me.

test (visualiser.visualiser_test.TestVisualiser) ... Loaded http://localhost:64400, status: success
[ OK ]  failed_info_test
[ OK ]  done_info_test
[ OK ]  upstream_failure_info_test
[ OK ]  result_count_test
[ OK ]  filtered_result_count_test1
[ OK ]  filtered_result_count_test2
[ OK ]  filtered_result_count_test3
[ OK ]  filtered_result_count_test4
[ OK ]  filtered_result_count_test5
[ OK ]  filtered_result_count_test5
[ OK ]  filtered_result_count_test5
[ OK ]  filtered_result_count_test5
[ OK ]  searched_result_count_test1
[ OK ]  searched_result_count_test1
RESULT: true
ok
test_keeps_entries_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_filter_on_server_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_hide_done_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_invert_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_order_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_table_filter_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_task_id_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_keeps_visualisation_type_after_page_refresh (visualiser.visualiser_test.TestVisualiser) ... ok
test_synchronizes_fields_on_graph_tab (visualiser.visualiser_test.TestVisualiser) ... ok
test_synchronizes_fields_on_tasks_tab (visualiser.visualiser_test.TestVisualiser) ... ok

----------------------------------------------------------------------
Ran 11 tests in 58.570s

OK
__________________________ summary __________________________
  visualiser: commands succeeded
  congratulations :)

@mention-bot
Copy link

@QuentinBrosse, thanks for your PR! By analyzing the history of the files in this pull request, we identified @daveFNbuck, @nmb10 and @riga to be potential reviewers.

Copy link
Contributor

@Tarrasch Tarrasch left a comment

Choose a reason for hiding this comment

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

Looks like a great start! :)

@QuentinBrosse
Copy link
Contributor Author

QuentinBrosse commented Aug 1, 2017

@Tarrasch Do you have more information about this ?

@dlstadther
Copy link
Collaborator

What's the PR state here? Is this ready-to-merge, or wip?

@Tarrasch Tarrasch merged commit 6e084b5 into spotify:master Aug 25, 2017
@Tarrasch
Copy link
Contributor

I recall this was ready to merge. :)

Thanks @QuentinBrosse and @dlstadther!

This was referenced Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants