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

coverage testing #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

coverage testing #38

wants to merge 2 commits into from

Conversation

bqpd
Copy link
Contributor

@bqpd bqpd commented Sep 2, 2019

No description provided.

@1ozturkbe
Copy link
Contributor

This is nice! Also, can we add one of those cool 'build passing' indicators?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

Yup! Did you find the coverage reports? There's cobertura and coverage.py

(thanks @galbramc!)

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

@galbramc
Copy link

galbramc commented Sep 2, 2019

You can get the build passing indicator links here:

https://acdl.mit.edu/csi/job/gpkit_ResearchModel_robust_Push/badge/

@galbramc
Copy link

galbramc commented Sep 2, 2019

Is this not considered a research model? Why do we have both robust_PullRequest job and gpkit_ResearchModel_robust_Push?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

I think this will be transitioning from a research model to having pylint, coverage tests etc., one step at a time.

@galbramc
Copy link

galbramc commented Sep 2, 2019

Ok so you want the same type of testing as you have for gpkit?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

I think so, but it's @1ozturkbe's show; Berk whaddya want?

@galbramc
Copy link

galbramc commented Sep 2, 2019

What is the purpose of this repo? Is it a different category than other research models?

Why don't we have lint and coverage information for ResearchModels?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

Because it doesn't work with the dynamic-import stuff they run tests with :-/

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

which, writing a python file to run all your tests isn't hard, but most of those repos are on life support as is.

@1ozturkbe
Copy link
Contributor

It's going to be a process, but yes, I'd like this repo to have the same type of testing as gpkit.

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

test this please

@galbramc
Copy link

galbramc commented Sep 2, 2019

Hmm, things are much easer to manage if we have families of jobs so we can use the job templates to manage things.

Would it be possible to unity things so so all repos run their respective tests in the same way? Can we update repos that needed and deprecate repos that are on life support?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

test this please

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

@galbramc yup, I'll look into using templates

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

@1ozturkbe looks like you're testing about half of your code right now, not too bad for the number of tests you have! Let me know if you want a copilot for this, else I'll just fix the Windows testing errors and leave it to ya.

image

@whoburg
Copy link
Collaborator

whoburg commented Sep 2, 2019

@galbramc no to deprecating the research models! @bqpd is certainly right that some of the repos are on life support, but meaning they need TLC and some effort to keep them up to date, not that they should go away.

@whoburg
Copy link
Collaborator

whoburg commented Sep 2, 2019

That's a cool screenshot @bqpd! Super useful tool.

@1ozturkbe
Copy link
Contributor

@bqpd I'm actually surprised as well about the coverage! I knew it was bad, and it seems like the last few PRs helped. I do have ideas for drastically improving coverage (>80%) since some of the functionalities have are completely untested. I also want the testing to somewhat like documentation, so robust can be a little more user friendly. But this is an awesome start!

assert(mm.substitutions.get(key) == m.substitutions.get(key) *
(1.+mm.setting.get("gamma")*key.pr/100.))
self.assertGreater(margin_solution['cost'], mm.nominal_cost)

Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't even noticed this... Definitely a merge issue, thanks @bqpd.

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

test this please

1 similar comment
@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

test this please

@galbramc
Copy link

galbramc commented Sep 2, 2019

I was only suggesting deprecating anything that is really not worth keeping.

So who is going to do the TLC to get the research models updated. In particular so they can be tested with coverage and pylint?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

test this please

1 similar comment
@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

test this please

@bqpd
Copy link
Contributor Author

bqpd commented Sep 2, 2019

@galbramc I think they're fine as they are, but it'll be me doing that if it gets done :p

@1ozturkbe
Copy link
Contributor

test this please

@1ozturkbe
Copy link
Contributor

test this please

@1ozturkbe
Copy link
Contributor

@galbramc @bqpd it seems like there is an issue with the installation of mosek. It's odd because the unit tests were passing yesterday.

@galbramc
Copy link

galbramc commented Sep 3, 2019

I don't know what changed, or what this error means:

ImportError: Could not load MOSEK library: ImportError('No module named lib',)

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019 via email

@galbramc
Copy link

galbramc commented Sep 4, 2019

gpkit_Push is failing the same way. I don't know what changed... I didn't do anything

@whoburg
Copy link
Collaborator

whoburg commented Sep 4, 2019

I don't think an out of date license file would manifest that way, but maybe worth double-checking?

@galbramc
Copy link

galbramc commented Sep 4, 2019

The license is checked above and it checks our cleanly.

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019

it's not finding mosek during installation...

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019

oh it's because it's not finding ctypesgencore

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019

ah, it's a new version of ctypesgen that's being pulled down by pip

@galbramc
Copy link

galbramc commented Sep 4, 2019

Ahh, and since gpkit_install does not expliclty test for mosek, it didn't fail when it found the new ctypesgen. One purpose of gkit_install is to sort out when dependencies break gpkit. Looking at the log I see:

## SKIPPING MOSEK INSTALL: CTYPESGENCORE WAS NOT FOUND
# Failed building mosek

Maybe gpkit_install should also explicitly run cvxopt and mosek?

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019 via email

@galbramc
Copy link

galbramc commented Sep 4, 2019

Yes, I think gpkit_install should have failed as well because ctypesgen didn't work. Do you not agree? This means you have a false positive saying you think gpkit from pip is working when in fact it does not work with mosek and the latest ctypesgen right now.

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019

oh, yeah that's true.

The problem right now is that anaconda (used on the windows machines) seems to come with the new version of ctypesgen, so it can't be uninstalled...seeing if I can fix that with --force

@bqpd
Copy link
Contributor Author

bqpd commented Sep 4, 2019

@galbramc that didn't work, so it now works on everything but windows... https://acdl.mit.edu/csi/job/gpkit_PullRequest_Unit/2820/

anyway I'll try to get it working with ctypesgen 1.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants