Skip to content

Add tests for all applications and limit mechanics explorer opening. #7

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.

Sign up for GitHub

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

Jump to bottom
Merged
H0R5E merged 10 commits into from
Aug 12, 2021
Merged

Add tests for all applications and limit mechanics explorer opening. #7

H0R5E merged 10 commits into from
Aug 12, 2021

Conversation

@H0R5E
Copy link
Contributor

@H0R5E H0R5E commented Jun 29, 2021 •
edited
Loading

This PR is an accompaniment to WEC-Sim/WEC-Sim#619 and WEC-Sim/WEC-Sim#620

Tests have been added to each level where a bemio call is made and a script to call them globally has also been added at the top level called wecSimAppTest.

I did also reduce the number of times that the mechanics explorer is opened, by default. Some way to programmatically stop this in tests would be useful.

Some instructions regarding running the tests (i.e. needed WEC-Sim and MoorDyn) have been added.

TODO:

@H0R5E H0R5E mentioned this pull request Jun 29, 2021
Closed
10 tasks
@akeeste
Copy link
Contributor

akeeste commented Jul 1, 2021

@H0R5E

Overall I think this PR looks good. Each application has its own test in the folder that prevents the previously discussed file copying and directory changes.

My two suggestions are to:

  • By default skip the desalination and MoorDyn tests (including Paraview RM3 with MoorDyn). These are not typically setup for the additional dependencies. Once the new version of MoorDyn is setup correctly with WEC-Sim, this can be turned on.
  • I added a line to the README.md instructing new users to create a test for their application

I was having trouble pushing this to your branch, but the line I placed before "Create a pull request..." is:
* Using the TestB2B.m test as a template, create a *TestCASE.m* file that will run BEMIO and check the functionality of your application when changes are made to the WEC-Sim source code.

@H0R5E
Copy link
Contributor Author

H0R5E commented Jul 2, 2021 •
edited
Loading

@akeeste, thanks for the feedback.

I'll see if I can get some automated skipping set up to check for MoorDyn, if it's required for the tests. I'd rather avoid trying to group the tests and adding flags, particularly if tests that use MoorDyn can be placed anywhere.

Did you try and push directly to my branch? That won't work because I've not granted you access rights to my fork. I'll add your edits to the README file. In the future you can always add a PR against my fork, like I did for the ci_restructure branch on your WEC-Sim fork, otherwise I can grant you push rights to my WEC-Sim_Applications fork - depends on the situation.

@H0R5E
Copy link
Contributor Author

H0R5E commented Jul 2, 2021 •
edited
Loading

OK, so I don't understand what is going on with the Desalination case. The error it chucks is:

================================================================================
Error occurred in TestDesalination/testDesalination and it did not run to completion.
    ---------
    Error ID:
    ---------
    'MATLAB:MException:MultipleErrors'
    --------------
    Error Details:
    --------------
    Error using wecSim (line 376)
    Error due to multiple causes.
    
    Error in TestDesalination/testDesalination (line 65)
                wecSim
    
    Caused by:
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Gas-Charged Accumulator'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Hydraulic Cylinder & Directional Valves/2-Way Directional Valve 1'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Hydraulic Cylinder & Directional Valves/2-Way Directional Valve 2'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Hydraulic Cylinder & Directional Valves/2-Way Directional Valve 3'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Hydraulic Cylinder & Directional Valves/2-Way Directional Valve 4'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Hydraulic Cylinder & Directional Valves/Double-Acting Hydraulic Cylinder
        (Simple)'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Pressure Relief Valve'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Pressure Exchanger/Fixed-Displacement Pump'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /Pressure Exchanger/Hydraulic Motor'.
        Error using wecSim (line 376)
        Failed to load library 'sh_lib' referenced by 'OSWEC_RO/Piston  &     RO
        /RO Membrane/Osmotic  Pressure'.
================================================================================
.
Done TestDesalination
__________

Failure Summary:

     Name                               Failed  Incomplete  Reason(s)
    ==================================================================
     TestDesalination/testDesalination    X         X       Errored.

ans = 

  TestResult with properties:

          Name: 'TestDesalination/testDesalination'
        Passed: 0
        Failed: 1
    Incomplete: 1
      Duration: 279.5456
       Details: [1×1 struct]

Totals:
   0 Passed, 1 Failed (rerun), 1 Incomplete.
   279.5456 seconds testing time.

What is sh_lib? Are we expecting a known failure here? I can get it to skip on that basis.

@akeeste
Copy link
Contributor

akeeste commented Jul 2, 2021

@H0R5E agreed, it would be better to avoid the flags all together. Right now I believe only a specific version of MoorDyn works with WEC-Sim. Yi-Hsiang is working with the MoorDyn team to get that resolved in the next month. The desalination case additionally requires the Simscape fluids toolbox, which is probably why its failing.

I think until this is all automated/scheduled, the tests can still move here given the team's awareness that the MoorDyn/Desal ones may fail. We can skip them on the schedule if needed until the dependencies are addressed.

@H0R5E H0R5E mentioned this pull request Jul 2, 2021
Merged
@H0R5E
Copy link
Contributor Author

H0R5E commented Jul 2, 2021

Waiting on WEC-Sim/WEC-Sim#632

@akeeste
Copy link
Contributor

akeeste commented Jul 21, 2021

@H0R5E WEC-Sim/WEC-Sim#632 is merged.

@H0R5E H0R5E marked this pull request as ready for review August 5, 2021 17:15
@H0R5E
Copy link
Contributor Author

H0R5E commented Aug 5, 2021

This is now ready for review

Aside: @kmruehl can you give me admin rights on this repo? I'll need it to set up the CI. Thanks.

@H0R5E H0R5E requested a review from akeeste August 6, 2021 17:01
@kmruehl kmruehl self-requested a review August 9, 2021 16:39
@akeeste akeeste mentioned this pull request Aug 11, 2021
Closed
akeeste
akeeste approved these changes Aug 11, 2021
View reviewed changes
Copy link
Contributor

@akeeste akeeste left a comment

Choose a reason for hiding this comment

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

No more suggestions that the current improvements already made on this PR. It will be very helpful to have merged. My only comment is that the WECCCOMP case explorer is still turned on. Is there any reason to add the test option parameter like we have in wecSimTest?

@H0R5E
Copy link
Contributor Author

H0R5E commented Aug 12, 2021

@akeeste, thanks. I have added a test skip for WEC-Sim/WEC-Sim#674 and I have switched off the explorer for WECCCOMP case. Everything passes for me now, with MoorDyn installed.

Regarding the test flags, I think the way I have it set up here is easier for this repository as you can just point the function at the directory you want to test. So like:

wecSimAppTest End_Stops

will run the tests in the End_Stops directory. We don't need to keep adding flags if people add new directories doing it this way.

I'm going to merge this now and start testing the CI.

@H0R5E H0R5E merged commit d622cd3 into WEC-Sim:master Aug 12, 2021
@H0R5E H0R5E deleted the test_suite branch August 12, 2021 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akeeste akeeste akeeste approved these changes

@kmruehl kmruehl Awaiting requested review from kmruehl

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@H0R5E @akeeste