Adding Retest

[tengis-nrl]

Thanks for opening a PR to PowerSystems.jl, please take note of the following when making a PR:

Check the contributor guidelines

1. Add a description of the changes proposed in the pull request.
2. Is my PR fixing an open issue? Add the reference to the related issue
3. If you are contributing a new struct: please refer to the testing requirements for new structs

[github-actions]

[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

[GabrielKS]

@tengis-nrl you should learn how to invoke the formatter set up in each Sienna repository so you can deal with these issues proactively. I can help you with that sometime if you need

[tengis-nrl]

How about during the next time we meet? Thanks!

[github-actions]

[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

	using .PowerSystemsTests
	using .PowerSystemsTests

[github-actions]

[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

	recursive_includet("PowerSystemsTests.jl")
	recursive_includet("PowerSystemsTests.jl")

[github-actions]

[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

	run_tests()
	run_tests()

[tengis-nrl]

Hi @GabrielKS, this file was added to use ReTest instead of the current testing package. There is a file called InfrastructureSystemsTests.jl in IS that has almost the same code as this one. @daniel-thom was wondering how we can avoid code duplication. Specifically, he asked "Is there a way to avoid that without making the ReTest.jl package a dependency of PSY? Keep in mind that there are dozens of Sienna packages that should use this test pattern." Any suggestion?

[GabrielKS]

Dan and I talked about this a while ago and the options I see are:

1. Put the stuff that would be duplicated into InfrastructureSystems.jl. Advantage: no massive code duplication across Sienna repos, just call the relevant thing from IS. Disadvantage: ReTest.jl would need to be a non-test dependency of IS (and therefore, transitively, of all the other Sienna packages)
2. Current approach: reimplement for every repository. Advantage: ReTest.jl can be just a test dependency. Disadvantage: massive code duplication.
3. Create a new package that is just for testing, put the stuff that would be duplicated there. Advantages: no massive duplication and ReTest is just a test dependency. Disadvantage: we have to maintain a whole separate package for this.
4. Same as 3 but with a submodule or something instead of a package. Same advantages as 3. Disadvantage: it's really annoying to work with submodules.

I don't think we're going to come up with anything that doesn't have some of those disadvantages. I might not fully appreciate the costs of maintaining a whole separate package, but option 3 looks the most attractive to me. We could also do something like this to remove the duplication of all our formatting scripts.

[jd-lara]

For the time being we will keep 2. as the option since it also creates a headache to maintain package dependencies to do any of the others.

[GabrielKS]

Copied from the IS PR: Seems like even if the best practice is to install Revise globally, if our tests are going to depend it we ought to have it in test/Project.toml

[jd-lara]

remove, we need to remove from IS too.

[daniel-thom]

This file is optional. Users don’t have to call it. I say leave it the way it is. The instructions should advise you to install TestEnv.jl and Revise.jl globally.

[GabrielKS]

Okay, that works for me

[GabrielKS]

Dan and I talked about this a while ago and the options I see are:

1. Put the stuff that would be duplicated into InfrastructureSystems.jl. Advantage: no massive code duplication across Sienna repos, just call the relevant thing from IS. Disadvantage: ReTest.jl would need to be a non-test dependency of IS (and therefore, transitively, of all the other Sienna packages)
2. Current approach: reimplement for every repository. Advantage: ReTest.jl can be just a test dependency. Disadvantage: massive code duplication.
3. Create a new package that is just for testing, put the stuff that would be duplicated there. Advantages: no massive duplication and ReTest is just a test dependency. Disadvantage: we have to maintain a whole separate package for this.
4. Same as 3 but with a submodule or something instead of a package. Same advantages as 3. Disadvantage: it's really annoying to work with submodules.

I don't think we're going to come up with anything that doesn't have some of those disadvantages. I might not fully appreciate the costs of maintaining a whole separate package, but option 3 looks the most attractive to me. We could also do something like this to remove the duplication of all our formatting scripts.

[GabrielKS]

@tengis-nrl you should learn how to invoke the formatter set up in each Sienna repository so you can deal with these issues proactively. I can help you with that sometime if you need

[daniel-thom]

I like option 3 the best. Good point about the formatter scripts. Hopefully, this won’t change often. Also, the formatter version would be fixed in this package.

[jd-lara]

I like option 3 the best. Good point about the formatter scripts. Hopefully, this won’t change often. Also, the formatter version would be fixed in this package.

@pesap suggested adding a modified GitHub action that can commit the required formatter changes.

[jd-lara]

@daniel-thom do you want to merge this or go ahead with the creation of the testing package?

[daniel-thom]

@daniel-thom do you want to merge this or go ahead with the creation of the testing package?

I think we should create SiennaDevUtils.jl, add these test functions and the formatter, and then change IS and PSY to use it.

[jd-lara]

closing since the preferred solution of creating a testing package will be implemented.