Moose Server unit tests enforce unrelated things
Bug Description
See this failure obtained after adding two valid parameters
[ RUN ] MooseServerTest.CompletionMeshDefaultedType
/data/civet2/build/moose/unit/src/MooseServerTest.C:1048: Failure
Expected: 48u
Which is: 48
To be equal to: completions_array.size()
Which is: 50
/data/civet2/build/moose/unit/src/MooseServerTest.C:1107: Failure
Expected: completions_expect
Which is: "\nlabel: active text: active = '${1:__all__}' desc: If specified only... pos: [6.0]-[6.0] kind: 7 format: snippet\nlabel: add_subdomain_ids text: add_subdomain_ids = desc: The listed subdom... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: add_subdomain_names text: add_subdomain_names = desc: Optional list of ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: allow_renumbering text: allow_renumbering = ${1:true} desc: If allow_renumber... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: alpha_rotation text: alpha_rotation = desc: The number of deg... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: beta_rotation text: beta_rotation = desc: The number of deg... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: block_id text: block_id = desc: IDs of the block ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: block_name text: block_name = desc: Names of the bloc... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: boundary_id text: boundary_id = desc: IDs of the bounda... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: boundary_name text: boundary_name = desc: Names of the boun... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: build_all_side_lowerd_mesh text: build_all_side_lowerd_mesh = ${1:false} desc: True to build the... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: centroid_partitioner_direction text: centroid_partitioner_direction = desc: Specifies the sor... pos: [6.0]-[6.0] kind: 13 format: regular\nlabel: clear_spline_nodes text: clear_spline_nodes = ${1:false} desc: If clear_spline_n... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: construct_node_list_from_side_list text: construct_node_list_from_side_list = ${1:true} desc: Whether or not to... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: construct_side_list_from_node_list text: construct_side_list_from_node_list = ${1:false} desc: If true, construc... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: control_tags text: control_tags = desc: Adds user-defined... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: coord_block text: coord_block = desc: Block IDs for the... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: coord_type text: coord_type = '${1:XYZ}' desc: Type of the coord... pos: [6.0]-[6.0] kind: 13 format: snippet\nlabel: enable text: enable = ${1:true} desc: Set the enabled s... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: file text: file = desc: The name of the m... pos: [6.0]-[6.0] kind: 23 format: regular\nlabel: gamma_rotation text: gamma_rotation = desc: The number of deg... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: ghosted_boundaries text: ghosted_boundaries = desc: Boundaries to be ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: ghosted_boundaries_inflation text: ghosted_boundaries_inflation = desc: If you are using ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: ghosting_patch_size text: ghosting_patch_size = desc: The number of nea... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: inactive text: inactive = desc: If specified bloc... pos: [6.0]-[6.0] kind: 7 format: regular\nlabel: include_local_in_ghosting text: include_local_in_ghosting = ${1:false} desc: Boolean used to t... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: length_unit text: length_unit = desc: How much distance... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: max_leaf_size text: max_leaf_size = ${1:10} desc: The maximum numbe... pos: [6.0]-[6.0] kind: 14 format: snippet\nlabel: nemesis text: nemesis = ${1:false} desc: If nemesis=true a... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: output_ghosting text: output_ghosting = ${1:false} desc: Boolean to turn o... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: partitioner text: partitioner = ${1:default} desc: Specifies a mesh ... pos: [6.0]-[6.0] kind: 13 format: snippet\nlabel: patch_size text: patch_size = ${1:40} desc: The number of nod... pos: [6.0]-[6.0] kind: 14 format: snippet\nlabel: rz_coord_axis text: rz_coord_axis = ${1:Y} desc: The rotation axis... pos: [6.0]-[6.0] kind: 13 format: snippet\nlabel: rz_coord_blocks text: rz_coord_blocks = desc: Blocks using gene... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: rz_coord_directions text: rz_coord_directions = desc: Axis directions f... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: rz_coord_origins text: rz_coord_origins = desc: Axis origin point... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: second_order text: second_order = ${1:false} desc: Converts a first ... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: skip_deletion_repartition_after_refine text: skip_deletion_repartition_after_refine = ${1:false} desc: If the flag is tr... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: skip_partitioning text: skip_partitioning = ${1:false} desc: If true the mesh ... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: skip_refine_when_use_split text: skip_refine_when_use_split = ${1:true} desc: True to skip unif... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: split_file text: split_file = desc: Optional name of ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: type text: type = ${1:FileMesh} desc: A string represen... pos: [6.0]-[6.0] kind: 25 format: snippet\nlabel: uniform_refine text: uniform_refine = ${1:0} desc: Specify the level... pos: [6.0]-[6.0] kind: 14 format: snippet\nlabel: up_direction text: up_direction = desc: Specify what axis... pos: [6.0]-[6.0] kind: 13 format: regular\nlabel: use_displaced_mesh text: use_displaced_mesh = ${1:true} desc: Create the displa... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: use_split text: use_split = ${1:false} desc: Use split distrib... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: * text: [block_name]\\n $0\\n[] desc: custom user named... pos: [6.0]-[6.0] kind: 6 format: snippet\nlabel: Partitioner text: [Partitioner]\\n $0\\n[] desc: application named... pos: [6.0]-[6.0] kind: 22 format: snippet\n"
To be equal to: "\n" + completions_actual.str()
Which is: "\nlabel: active text: active = '${1:__all__}' desc: If specified only... pos: [6.0]-[6.0] kind: 7 format: snippet\nlabel: add_sideset_ids text: add_sideset_ids = desc: The listed sidese... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: add_sideset_names text: add_sideset_names = desc: The listed sidese... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: add_subdomain_ids text: add_subdomain_ids = desc: The listed subdom... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: add_subdomain_names text: add_subdomain_names = desc: Optional list of ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: allow_renumbering text: allow_renumbering = ${1:true} desc: If allow_renumber... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: alpha_rotation text: alpha_rotation = desc: The number of deg... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: beta_rotation text: beta_rotation = desc: The number of deg... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: block_id text: block_id = desc: IDs of the block ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: block_name text: block_name = desc: Names of the bloc... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: boundary_id text: boundary_id = desc: IDs of the bounda... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: boundary_name text: boundary_name = desc: Names of the boun... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: build_all_side_lowerd_mesh text: build_all_side_lowerd_mesh = ${1:false} desc: True to build the... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: centroid_partitioner_direction text: centroid_partitioner_direction = desc: Specifies the sor... pos: [6.0]-[6.0] kind: 13 format: regular\nlabel: clear_spline_nodes text: clear_spline_nodes = ${1:false} desc: If clear_spline_n... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: construct_node_list_from_side_list text: construct_node_list_from_side_list = ${1:true} desc: Whether or not to... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: construct_side_list_from_node_list text: construct_side_list_from_node_list = ${1:false} desc: If true, construc... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: control_tags text: control_tags = desc: Adds user-defined... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: coord_block text: coord_block = desc: Block IDs for the... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: coord_type text: coord_type = '${1:XYZ}' desc: Type of the coord... pos: [6.0]-[6.0] kind: 13 format: snippet\nlabel: enable text: enable = ${1:true} desc: Set the enabled s... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: file text: file = desc: The name of the m... pos: [6.0]-[6.0] kind: 23 format: regular\nlabel: gamma_rotation text: gamma_rotation = desc: The number of deg... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: ghosted_boundaries text: ghosted_boundaries = desc: Boundaries to be ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: ghosted_boundaries_inflation text: ghosted_boundaries_inflation = desc: If you are using ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: ghosting_patch_size text: ghosting_patch_size = desc: The number of nea... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: inactive text: inactive = desc: If specified bloc... pos: [6.0]-[6.0] kind: 7 format: regular\nlabel: include_local_in_ghosting text: include_local_in_ghosting = ${1:false} desc: Boolean used to t... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: length_unit text: length_unit = desc: How much distance... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: max_leaf_size text: max_leaf_size = ${1:10} desc: The maximum numbe... pos: [6.0]-[6.0] kind: 14 format: snippet\nlabel: nemesis text: nemesis = ${1:false} desc: If nemesis=true a... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: output_ghosting text: output_ghosting = ${1:false} desc: Boolean to turn o... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: partitioner text: partitioner = ${1:default} desc: Specifies a mesh ... pos: [6.0]-[6.0] kind: 13 format: snippet\nlabel: patch_size text: patch_size = ${1:40} desc: The number of nod... pos: [6.0]-[6.0] kind: 14 format: snippet\nlabel: rz_coord_axis text: rz_coord_axis = ${1:Y} desc: The rotation axis... pos: [6.0]-[6.0] kind: 13 format: snippet\nlabel: rz_coord_blocks text: rz_coord_blocks = desc: Blocks using gene... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: rz_coord_directions text: rz_coord_directions = desc: Axis directions f... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: rz_coord_origins text: rz_coord_origins = desc: Axis origin point... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: second_order text: second_order = ${1:false} desc: Converts a first ... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: skip_deletion_repartition_after_refine text: skip_deletion_repartition_after_refine = ${1:false} desc: If the flag is tr... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: skip_partitioning text: skip_partitioning = ${1:false} desc: If true the mesh ... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: skip_refine_when_use_split text: skip_refine_when_use_split = ${1:true} desc: True to skip unif... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: split_file text: split_file = desc: Optional name of ... pos: [6.0]-[6.0] kind: 14 format: regular\nlabel: type text: type = ${1:FileMesh} desc: A string represen... pos: [6.0]-[6.0] kind: 25 format: snippet\nlabel: uniform_refine text: uniform_refine = ${1:0} desc: Specify the level... pos: [6.0]-[6.0] kind: 14 format: snippet\nlabel: up_direction text: up_direction = desc: Specify what axis... pos: [6.0]-[6.0] kind: 13 format: regular\nlabel: use_displaced_mesh text: use_displaced_mesh = ${1:true} desc: Create the displa... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: use_split text: use_split = ${1:false} desc: Use split distrib... pos: [6.0]-[6.0] kind: 8 format: snippet\nlabel: * text: [block_name]\\n $0\\n[] desc: custom user named... pos: [6.0]-[6.0] kind: 6 format: snippet\nlabel: Partitioner text: [Partitioner]\\n $0\\n[] desc: application named... pos: [6.0]-[6.0] kind: 22 format: snippet\n"
With diff:
@@ +1,6 @@
label: active text: active = '${1:__all__}' desc: If specified only... pos: [6.0]-[6.0] kind: 7 format: snippet
+label: add_sideset_ids text: add_sideset_ids = desc: The listed sidese... pos: [6.0]-[6.0] kind: 14 format: regular
+label: add_sideset_names text: add_sideset_names = desc: The listed sidese... pos: [6.0]-[6.0] kind: 14 format: regular
label: add_subdomain_ids text: add_subdomain_ids = desc: The listed subdom... pos: [6.0]-[6.0] kind: 14 format: regular
label: add_subdomain_names text: add_subdomain_names = desc: Optional list of ... pos: [6.0]-[6.0] kind: 14 format: regular
[ FAILED ] MooseServerTest.CompletionMeshDefaultedType (11 ms)
Steps to Reproduce
Modify any class (moosemesh) or input feature that is caught in these unit tests and hardcoded to never change
Impact
This is slowing down other developments.
[Optional] Diagnostics
No response
@brandonlangley
@brandonlangley can you generalize the test to allow for new params like we've done with some of the other tests?
I ll address the MooseMesh parameter one for now. Thanks!
I thought we had an issue for this but I'm too lazy to search rn
ah yes I opened https://github.com/idaholab/moose/issues/26033 then we closed it after two fixes. I didnt realize there were more issues at the time
@loganharbour - I just updated the MooseServerTest in #27924 so it will no longer fail when new input syntax is added.
@GiudGiud - Depending on if my #27924 or your #27914 is merged first, one of us will resolve a conflict in that test file.
This is still a problem. The test still enforces the mesh parameter descriptions see this failure https://civet.inl.gov/job/2293872/
@GiudGiud -
But this one is a little different, right? It was not caused by adding any new parameters or blocks.
It was due to the existing add_subdomain_names parameter having its description changed from:
Optional list of subdomain names to be applied to the ids given in add_subdomain_ids...
to now be:
The listed subdomain names will be assumed valid for the mesh...
While still technically unrelated, this will be a much more rare case than adding new input pieces.
I'm not sure how to test the server (that assists MOOSE input) without testing a few input pieces.
But it should rarely happen now and only need a small update when a tested piece gets changed.
But this one is a little different, right? It was not caused by adding any new parameters or blocks.
yes you're right. and in fact as you keep improving the test it will always be something a little different.
I'm not sure how to test the server (that assists MOOSE input) without testing a few input pieces.
maybe it could be using pieces that are not supposed to change? like test objects. Or new objects created within the unit tests.
But it should rarely happen now and only need a small update when a tested piece gets changed.
I did hit this like 1 week after than the second fix so I would not say it's rare yet. It is a totally small update and takes me like 2 minutes at most. Unfortunately most people are not familiar with our unit tests
I just hit a server unit test failure in https://github.com/idaholab/moose/pull/27687 where I'm adding a new TimeIntegrators syntax. Just based off the CIVET results alone, I have no idea how to resolve this. It feels like every time I introduce new syntax or something akin I get an opaque error from the language server testing that takes a while to elucidate. It's frustrating
/data/civet0/build/moose/unit/src/MooseServerTest.C:334: Failure
Expected: (actual_items.str().find(line)) != (std::string::npos), actual: 18446744073709551615 vs 18446744073709551615
/data/civet0/build/moose/unit/src/MooseServerTest.C:334: Failure
Expected: (actual_items.str().find(line)) != (std::string::npos), actual: 18446744073709551615 vs 18446744073709551615
/data/civet0/build/moose/unit/src/MooseServerTest.C:334: Failure
Expected: (actual_items.str().find(line)) != (std::string::npos), actual: 18446744073709551615 vs 18446744073709551615
[ FAILED ] MooseServerTest.CompletionPartialInputCases (8 ms)
If we're going to keep the current tests, I think we need some kind of guide in our "contributing to MOOSE" documentation that explains how to modify these tests when introducing new syntax
My preference would be to remove the current style of testing however