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

MooseServerTest unit test are enforcing unrelated items #26033

Closed
GiudGiud opened this issue Nov 11, 2023 · 2 comments
Closed

MooseServerTest unit test are enforcing unrelated items #26033

GiudGiud opened this issue Nov 11, 2023 · 2 comments
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 Nov 11, 2023

Bug Description

See these two:

/data/civet2/build/moose/unit/src/MooseServerTest.C:596: Failure
      Expected: diagnostics_expect
      Which is: "\nline:18 column:0 - (BCs/all/boundary):\nline:18 column:0 -     the following side sets (ids) do not exist on the mesh: top (2), bottom (3)\nline:18 column:0 -     MOOSE distinguishes between \"node sets\" and \"side sets\" depending on whether\nline:18 column:0 -     you are using \"Nodal\" or \"Integrated\" BCs respectively. Node sets corresponding\nline:18 column:0 -     to your side sets are constructed for you by default.\nline:18 column:0 -     Try setting \"Mesh/construct_side_list_from_node_list=true\" if you see this error.\nline:18 column:0 -     Note: If you are running with adaptivity you should prefer using side sets.\n"
To be equal to: "\n" + diagnostics_actual.str()
      Which is: "\nline:18 column:0 - (BCs/all/boundary):\nline:18 column:0 -     the following side sets (ids) do not exist on the mesh: top (2), bottom (3)\nline:18 column:0 -     MOOSE distinguishes between \"node sets\" and \"side sets\" depending on whether \nline:18 column:0 -     you are using \"Nodal\" or \"Integrated\" BCs respectively. Node sets corresponding \nline:18 column:0 -     to your side sets are constructed for you by default.\nline:18 column:0 -     Try setting \"Mesh/construct_side_list_from_node_list=true\" if you see this error.\nline:18 column:0 -     Note: If you are running with adaptivity you should prefer using side sets.\n"
With diff:
@@ -2,6 +2,6 @@
 line:18 column:0 - (BCs/all/boundary):
 line:18 column:0 -     the following side sets (ids) do not exist on the mesh: top (2), bottom (3)
-line:18 column:0 -     MOOSE distinguishes between \"node sets\" and \"side sets\" depending on whether
-line:18 column:0 -     you are using \"Nodal\" or \"Integrated\" BCs respectively. Node sets corresponding
+line:18 column:0 -     MOOSE distinguishes between \"node sets\" and \"side sets\" depending on whether 
+line:18 column:0 -     you are using \"Nodal\" or \"Integrated\" BCs respectively. Node sets corresponding 
 line:18 column:0 -     to your side sets are constructed for you by default.
 line:18 column:0 -     Try setting \"Mesh/construct_side_list_from_node_list=true\" if you see this error.

[  FAILED  ] MooseServerTest.DocumentChangeAndDiagnostics (19 ms)
[ RUN      ] MooseServerTest.CompletionDocumentRootLevel
/data/civet2/build/moose/unit/src/MooseServerTest.C:936: Failure
      Expected: 46u
      Which is: 46
To be equal to: completions_array.size()
      Which is: 47
[  FAILED  ] MooseServerTest.CompletionDocumentRootLevel (0 ms)

The first one is likely because I formatted the test file by saving in VSCode
the second one is enforcing that the number of systems remains unchanged (after I already adapted a test that was enforcing the list of systems)

Enforcing formatting and enforcing things about the systems in MOOSE are not good ideas and unit tests ought to be more targetted than this.

Steps to Reproduce

Add a new system to MOOSE

Impact

Waste time dealing with unit tests failures

@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 Nov 11, 2023
GiudGiud added a commit to GiudGiud/moose that referenced this issue Dec 30, 2023
GiudGiud added a commit to GiudGiud/moose that referenced this issue Dec 30, 2023
GiudGiud added a commit to GiudGiud/moose that referenced this issue Jan 10, 2024
GiudGiud added a commit to GiudGiud/moose that referenced this issue Jan 10, 2024
GiudGiud added a commit to GiudGiud/moose that referenced this issue Jan 23, 2024
GiudGiud added a commit to GiudGiud/moose that referenced this issue Jan 23, 2024
GiudGiud added a commit to GiudGiud/moose that referenced this issue Jan 23, 2024
GiudGiud added a commit to GiudGiud/moose that referenced this issue Jan 23, 2024
@brandonlangley
Copy link
Contributor

@GiudGiud -

Thanks a lot for opening this issue and also for updating the test to no longer enforce the list of systems in 70297ec.

I also added a commit that updated the test to trim trailing whitespace and no longer enforce formatting in ce24f05.

With your commit being merged yesterday and my commit being merged in December, this issue can be closed now.

@GiudGiud
Copy link
Contributor Author

I agree. Thanks for the whitespace fix.

Closing as resolved by 70297ec and ce24f05

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

No branches or pull requests

2 participants