moose icon indicating copy to clipboard operation
moose copied to clipboard

Moose Server unit tests enforce unrelated things

Open GiudGiud opened this issue 1 year ago • 9 comments

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

GiudGiud avatar Jun 17 '24 21:06 GiudGiud

@brandonlangley

GiudGiud avatar Jun 17 '24 21:06 GiudGiud

@brandonlangley can you generalize the test to allow for new params like we've done with some of the other tests?

loganharbour avatar Jun 17 '24 21:06 loganharbour

I ll address the MooseMesh parameter one for now. Thanks!

GiudGiud avatar Jun 17 '24 21:06 GiudGiud

I thought we had an issue for this but I'm too lazy to search rn

lindsayad avatar Jun 17 '24 21:06 lindsayad

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

GiudGiud avatar Jun 17 '24 21:06 GiudGiud

@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.

brandonlangley avatar Jun 18 '24 05:06 brandonlangley

This is still a problem. The test still enforces the mesh parameter descriptions see this failure https://civet.inl.gov/job/2293872/

GiudGiud avatar Jun 25 '24 17:06 GiudGiud

@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.

brandonlangley avatar Jun 25 '24 18:06 brandonlangley

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

GiudGiud avatar Jun 25 '24 19:06 GiudGiud

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)

lindsayad avatar Sep 24 '24 23:09 lindsayad

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

lindsayad avatar Sep 24 '24 23:09 lindsayad

My preference would be to remove the current style of testing however

lindsayad avatar Sep 24 '24 23:09 lindsayad