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

clean up tests to use FactCheck #47

Merged
merged 8 commits into from
Jan 17, 2015
Merged

clean up tests to use FactCheck #47

merged 8 commits into from
Jan 17, 2015

Conversation

davidlizeng
Copy link
Contributor

Reorganized test.jl and test2.jl into affine, lp, and socp test files as per #42 .
All tests also now use the FactCheck package.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling 077e892 on clean_tests into 5171206 on master.

@karanveerm
Copy link
Contributor

Few issues:

@coveralls
Copy link

Coverage Status

Coverage increased (+6.89%) when pulling ca24f9f on clean_tests into 5171206 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-47.93%) when pulling d8fd557 on clean_tests into 5171206 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+12.94%) when pulling 196040b on clean_tests into 5171206 on master.

@karanveerm
Copy link
Contributor

@davidlizeng I think this is ready to merge into master. Coverage should go up another 20% after we test with SCS on travis, after which most of the uncovered lines are error checks, etc. of lower priority.

@@ -1,3 +1,4 @@
julia 0.3
MathProgBase 0.3.8
ECOS 0.4.1
FactCheck 0.2.5
Copy link
Member

Choose a reason for hiding this comment

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

This is usually listed in test/REQUIRE since it's only needed to 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.

What would be the nicest way for FactCheck to still be present for travis?
JuMP seems to be doing this: https://github.com/JuliaOpt/JuMP.jl/blob/master/.travis.yml#L11

Copy link
Member

Choose a reason for hiding this comment

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

Oops, I guess we never tidied that up. @IainNZ
If you just use Pkg.test("Convex", coverage=true) then first the testing dependencies will be installed and then test/runtests.jl will be run. That's the "right" way to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done, thanks! I'm going to keep this PR open for another few days to see if anyone has other feedback, and merge it then.

@coveralls
Copy link

Coverage Status

Coverage increased (+17.61%) when pulling b8de06e on clean_tests into 5171206 on master.

karanveerm added a commit that referenced this pull request Jan 17, 2015
clean up tests to use FactCheck
@karanveerm karanveerm merged commit e2f930b into master Jan 17, 2015
@karanveerm karanveerm mentioned this pull request Jan 19, 2015
@ericphanson ericphanson deleted the clean_tests branch August 13, 2019 22:34
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.

4 participants