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.

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

Adding Retest #1052

Closed
wants to merge 3 commits into from
Closed

Adding Retest #1052

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5727fa6
Adding ReTest and relevant files
tengis-nrl Feb 13, 2024
a85317b
Adding a block of code back
tengis-nrl Feb 14, 2024
66de7e4
Ran the formatter
tengis-nrl Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions test/PowerSystemsTests.jl
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
module PowerSystemsTests

import TerminalLoggers: TerminalLogger
using ReTest
using Logging
using DataStructures
using Dates
using LinearAlgebra
using PowerSystemCaseBuilder
import TimeSeries
import InteractiveUtils
import JSON3
import PowerSystemCaseBuilder: PSYTestSystems
import InfrastructureSystems
const IS = InfrastructureSystems
using PowerSystems
import PowerSystems: PowerSystemTableData
const PSY = PowerSystems
const PSB = PowerSystemCaseBuilder

import Aqua
Aqua.test_unbound_args(PowerSystems)
Aqua.test_undefined_exports(PowerSystems)
Aqua.test_ambiguities(PowerSystems)
Aqua.test_stale_deps(PowerSystems)
Aqua.test_deps_compat(PowerSystems)

const BASE_DIR = dirname(dirname(Base.find_package("PowerSystems")))
const DATA_DIR = PSB.DATA_DIR
const TIME_SERIES_DIR = joinpath(DATA_DIR, "forecasts")
const MATPOWER_DIR = joinpath(DATA_DIR, "matpower")
const PSSE_RAW_DIR = joinpath(DATA_DIR, "psse_raw")
const PSSE_DYR_DIR = joinpath(DATA_DIR, "psse_dyr")
const PSSE_TEST_DIR = joinpath(DATA_DIR, "PSSE_test")
const RTS_GMLC_DIR = joinpath(DATA_DIR, "RTS_GMLC")
const TAMU_DIR = joinpath(DATA_DIR, "ACTIVSg2000")
const DESCRIPTORS = joinpath(RTS_GMLC_DIR, "user_descriptors.yaml")
const BAD_DATA = joinpath(DATA_DIR, "bad_data_for_tests")

LOG_FILE = "power-systems.log"
LOG_LEVELS = Dict(
"Debug" => Logging.Debug,
"Info" => Logging.Info,
"Warn" => Logging.Warn,
"Error" => Logging.Error,
)

include("common.jl")

function get_logging_level_from_env(env_name::String, default)
level = get(ENV, env_name, default)
return IS.get_logging_level(level)
end

logger = global_logger()

function run_tests(args...; kwargs...)
logger = global_logger()
try
logging_config_filename = get(ENV, "SIENNA_LOGGING_CONFIG", nothing)
if logging_config_filename !== nothing
config = IS.LoggingConfiguration(logging_config_filename)
else
config = IS.LoggingConfiguration(;
filename = LOG_FILE,
file_level = get_logging_level_from_env("SIENNA_FILE_LOG_LEVEL", "Info"),
console_level = get_logging_level_from_env(
"SIENNA_CONSOLE_LOG_LEVEL",
"Error",
),
)
end
console_logger = TerminalLogger(config.console_stream, config.console_level)

IS.open_file_logger(config.filename, config.file_level) do file_logger
levels = (Logging.Info, Logging.Warn, Logging.Error)
multi_logger =
IS.MultiLogger([console_logger, file_logger], IS.LogEventTracker(levels))
global_logger(multi_logger)

if !isempty(config.group_levels)
IS.set_group_levels!(multi_logger, config.group_levels)
end

@time retest(args...; kwargs...)
@test length(IS.get_log_events(multi_logger.tracker, Logging.Error)) == 0
@info IS.report_log_summary(multi_logger)
end
finally
# Guarantee that the global logger is reset.
global_logger(logger)
nothing
end
end


Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change

Copy link
Collaborator

Choose a reason for hiding this comment

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

@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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about during the next time we meet? Thanks!

export run_tests
@show "hello0"
end

using .PowerSystemsTests
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
using .PowerSystemsTests
using .PowerSystemsTests

2 changes: 2 additions & 0 deletions test/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ NLsolve = "2774e3e8-f4cf-5e23-947b-6d7e65073b56"
PowerSystemCaseBuilder = "f00506e0-b84f-492a-93c2-c0a9afc4364e"
PowerSystems = "bcd98974-b02a-5e2f-9ee0-a103f5c450dd"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
ReTest = "e0db7c4e-2690-44b9-bad6-7687da720f89"
SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
TerminalLoggers = "5d786b92-1e48-4d6f-9151-6b4477ca9bed"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
TimeSeries = "9e3dc215-6440-5c97-bce1-76c03772f85e"
UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
Expand Down
13 changes: 13 additions & 0 deletions test/load_tests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using Revise
Copy link
Collaborator

@GabrielKS GabrielKS Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

remove, we need to remove from IS too.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, that works for me


# Copied from https://juliatesting.github.io/ReTest.jl/stable/#Working-with-Revise
function recursive_includet(filename)
already_included = copy(Revise.included_files)
includet(filename)
newly_included = setdiff(Revise.included_files, already_included)
for (mod, file) in newly_included
Revise.track(mod, file)
end
end

recursive_includet("PowerSystemsTests.jl")
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
recursive_includet("PowerSystemsTests.jl")
recursive_includet("PowerSystemsTests.jl")

135 changes: 2 additions & 133 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,135 +1,4 @@
using Test
using Logging
using DataStructures
using Dates
using LinearAlgebra
using PowerSystemCaseBuilder
import TimeSeries
import InteractiveUtils
import JSON3
import PowerSystemCaseBuilder: PSYTestSystems
import InfrastructureSystems
const IS = InfrastructureSystems
using PowerSystems
import PowerSystems: PowerSystemTableData
const PSY = PowerSystems
const PSB = PowerSystemCaseBuilder

import Aqua
Aqua.test_unbound_args(PowerSystems)
Aqua.test_undefined_exports(PowerSystems)
Aqua.test_ambiguities(PowerSystems)
Aqua.test_stale_deps(PowerSystems)
Aqua.test_deps_compat(PowerSystems)

const BASE_DIR = dirname(dirname(Base.find_package("PowerSystems")))
const DATA_DIR = PSB.DATA_DIR
const TIME_SERIES_DIR = joinpath(DATA_DIR, "forecasts")
const MATPOWER_DIR = joinpath(DATA_DIR, "matpower")
const PSSE_RAW_DIR = joinpath(DATA_DIR, "psse_raw")
const PSSE_DYR_DIR = joinpath(DATA_DIR, "psse_dyr")
const PSSE_TEST_DIR = joinpath(DATA_DIR, "PSSE_test")
const RTS_GMLC_DIR = joinpath(DATA_DIR, "RTS_GMLC")
const TAMU_DIR = joinpath(DATA_DIR, "ACTIVSg2000")
const DESCRIPTORS = joinpath(RTS_GMLC_DIR, "user_descriptors.yaml")
const BAD_DATA = joinpath(DATA_DIR, "bad_data_for_tests")

LOG_FILE = "power-systems.log"
LOG_LEVELS = Dict(
"Debug" => Logging.Debug,
"Info" => Logging.Info,
"Warn" => Logging.Warn,
"Error" => Logging.Error,
)

include("common.jl")

"""
Copied @includetests from https://github.com/ssfrr/TestSetExtensions.jl.
Ideally, we could import and use TestSetExtensions. Its functionality was broken by changes
in Julia v0.7. Refer to https://github.com/ssfrr/TestSetExtensions.jl/pull/7.
"""

"""
Includes the given test files, given as a list without their ".jl" extensions.
If none are given it will scan the directory of the calling file and include all
the julia files.
"""
macro includetests(testarg...)
if length(testarg) == 0
tests = []
elseif length(testarg) == 1
tests = testarg[1]
else
error("@includetests takes zero or one argument")
end

quote
tests = $tests
rootfile = @__FILE__
if length(tests) == 0
tests = readdir(dirname(rootfile))
tests = filter(
f ->
startswith(f, "test_") && endswith(f, ".jl") && f != basename(rootfile),
tests,
)
else
tests = map(f -> string(f, ".jl"), tests)
end
println()
for test in tests
print(splitext(test)[1], ": ")
include(test)
println()
end
end
end

function get_logging_level_from_env(env_name::String, default)
level = get(ENV, env_name, default)
return IS.get_logging_level(level)
end

function run_tests()
logging_config_filename = get(ENV, "SIIP_LOGGING_CONFIG", nothing)
if logging_config_filename !== nothing
config = IS.LoggingConfiguration(logging_config_filename)
else
config = IS.LoggingConfiguration(;
filename = LOG_FILE,
file_level = get_logging_level_from_env("SIIP_FILE_LOG_LEVEL", "Info"),
console_level = get_logging_level_from_env("SIIP_CONSOLE_LOG_LEVEL", "Error"),
)
end
console_logger = ConsoleLogger(config.console_stream, config.console_level)

IS.open_file_logger(config.filename, config.file_level) do file_logger
levels = (Logging.Info, Logging.Warn, Logging.Error)
multi_logger =
IS.MultiLogger([console_logger, file_logger], IS.LogEventTracker(levels))
global_logger(multi_logger)

if !isempty(config.group_levels)
IS.set_group_levels!(multi_logger, config.group_levels)
end

# Testing Topological components of the schema
@time @testset "Begin PowerSystems tests" begin
@includetests ARGS
end

@test length(IS.get_log_events(multi_logger.tracker, Logging.Error)) == 0
@info IS.report_log_summary(multi_logger)
end
end

logger = global_logger()

try
run_tests()
finally
# Guarantee that the global logger is reset.
global_logger(logger)
nothing
end
include("PowerSystemsTests.jl")
run_tests()
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
run_tests()
run_tests()