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

Moose Server unit tests enforce unrelated things #27919

Open
GiudGiud opened this issue Jun 17, 2024 · 9 comments · Fixed by #27924
Open

Moose Server unit tests enforce unrelated things #27919

GiudGiud opened this issue Jun 17, 2024 · 9 comments · Fixed by #27924
Labels
P: normal A defect affecting operation with a low possibility of significantly affects. T: defect An anomaly, which is anything that deviates from expectations.

Comments

@GiudGiud
Copy link
Contributor

GiudGiud commented Jun 17, 2024

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 GiudGiud added T: defect An anomaly, which is anything that deviates from expectations. P: normal A defect affecting operation with a low possibility of significantly affects. labels Jun 17, 2024
@GiudGiud
Copy link
Contributor Author

@brandonlangley

@loganharbour
Copy link
Member

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

@GiudGiud
Copy link
Contributor Author

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

@lindsayad
Copy link
Member

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

@GiudGiud
Copy link
Contributor Author

ah yes I opened #26033 then we closed it after two fixes. I didnt realize there were more issues at the time

@brandonlangley
Copy link
Contributor

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

@GiudGiud
Copy link
Contributor Author

GiudGiud commented Jun 25, 2024

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

GiudGiud added a commit to grunerjmeier/moose that referenced this issue Jun 25, 2024
@brandonlangley
Copy link
Contributor

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

@GiudGiud
Copy link
Contributor Author

GiudGiud commented Jun 25, 2024

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 added a commit to grunerjmeier/moose that referenced this issue Jun 25, 2024
grunerjmeier pushed a commit to grunerjmeier/moose that referenced this issue Jun 26, 2024
abanki1 pushed a commit to abanki1/moose that referenced this issue Jul 11, 2024
abanki1 pushed a commit to abanki1/moose that referenced this issue Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: normal A defect affecting operation with a low possibility of significantly affects. T: defect An anomaly, which is anything that deviates from expectations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants