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

Refactor files in test/ so they can be run independently #2279

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

odow
Copy link
Member

@odow odow commented Jul 13, 2020

  • Make it so individual test files can be run
  • Time which tests take the most time
    • Currently, MutableArithmetics is the worst offender
  • Remove unneeded test entries in Project.toml

Refactored out of #2277

- Make it so individual test files can be run
- Time which tests take the most time
  - Currently, MutableArithmetics is the worst offender
- Remove unneeded test entries in Project.toml
x_dual = [DualNumbers.Dual(x[i],1.0) for i in 1:length(x)]
_epsilon(x::ForwardDiff.Dual{Nothing, Float64, 1}) = x.partials[1]

forward_dual_storage = zeros(ForwardDiff.Dual{Nothing, Float64, 1},length(nd))
Copy link
Member

Choose a reason for hiding this comment

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

Great, it was always complaining that I didn't have DualNumbers because I was not running the tests with ] test.

@codecov
Copy link

codecov bot commented Jul 13, 2020

Codecov Report

Merging #2279 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2279   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files          42       42           
  Lines        4234     4234           
=======================================
  Hits         3864     3864           
  Misses        370      370           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1129e58...cdfe63e. Read the comment docs.

@odow
Copy link
Member Author

odow commented Jul 15, 2020

Objections to merging? Once this is in I can start teeing up the rest of the Test PRs.


include(joinpath(@__DIR__, "utilities.jl"))

@static if !(:JuMPExtension in names(Main))
Copy link
Member

Choose a reason for hiding this comment

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

Why do we do this unusual check for JuMPExtension.jl but not utilities.jl?

Copy link
Member Author

@odow odow Jul 15, 2020

Choose a reason for hiding this comment

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

Utilities just loads some functions, whereas JuMPExtension is a module so we get annoying WARNING: module JuMPExtension has been redefined warnings.

The other PRs fix this by making each file a module, removing the need for the check. The module approach also prevents state leaking between files, and allows us to programmatically determine the tests to run within a particular file.

@odow odow merged commit a125a8f into master Jul 15, 2020
@odow odow deleted the od/isolate_tests branch July 15, 2020 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants