Skip to content
This repository has been archived by the owner on Jul 29, 2020. It is now read-only.

Handlers missing tests #89

Open
20 of 21 tasks
wasade opened this issue Aug 23, 2016 · 32 comments
Open
20 of 21 tasks

Handlers missing tests #89

wasade opened this issue Aug 23, 2016 · 32 comments

Comments

@wasade
Copy link
Member

wasade commented Aug 23, 2016

cc @EmbrietteH

@sjanssen2
Copy link
Collaborator

I tried to write tests for ag_edit_participant.py but I failed to open the according website on my browser, even after creating an admin user. Is this class unused?

@wasade
Copy link
Member Author

wasade commented Sep 17, 2016

It's still a working page from what I can see. You can access it via search, and edit. To go direct, you need to specify an email as an argument

@mortonjt
Copy link
Contributor

I noticed that ag_stats.py is already tested here and ag_third_party.py is tested here.

Are these tests considered inadequate, or should they be taken off of this list

@wasade
Copy link
Member Author

wasade commented Sep 19, 2016

From a quick glance, there seems to be a bunch of stuff there. What are you
thoughts?

On Mon, Sep 19, 2016 at 3:15 PM, Jamie Morton notifications@github.com
wrote:

I noticed that ag_stats.py is already tested here
https://github.com/biocore/labadmin/blob/master/knimin/tests/test_ag_stats.py
and ag_third_party.py is tested here
https://github.com/biocore/labadmin/blob/master/knimin/tests/test_third_party.py
.

Are these tests considered inadequate, or should they be taken off of this
list


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAc8svAUlbrlvk0sfk8asjwhFN8ivywfks5qrwmMgaJpZM4JrbSH
.

@mortonjt
Copy link
Contributor

From what I can tell, it looks like these two modules are already tested ...

@wasade
Copy link
Member Author

wasade commented Sep 19, 2016

kk

On Mon, Sep 19, 2016 at 4:07 PM, Jamie Morton notifications@github.com
wrote:

From what I can tell, it looks like these two modules are already tested
...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAc8skoRBCT8MPj9JWG9oi0ebZ9TaRbzks5qrxWrgaJpZM4JrbSH
.

@sjanssen2
Copy link
Collaborator

I redesigned tests for ag_stats.py because it was testing against hard coded numbers. Now it compares to stats obtained from database queries. It is part of my PR #111 , which fails otherwise, since it makes unrevertable changes to the DB.

@sjanssen2
Copy link
Collaborator

I'd like to know which lines of the code are not covered by my tests. I think this is normally possible with the coveralls page, but for this project it says: "The owner of this repo needs to re-authorize with github; their OAuth credentials are no longer valid so the file cannot be pulled from the github API.". Can we change this?

@wasade
Copy link
Member Author

wasade commented Sep 20, 2016

@sjanssen2, I clicked on one of the coverage links and it seemed to work? See here

@sjanssen2
Copy link
Collaborator

@wasade You are right for the general table, but if you click on one of the listed files https://coveralls.io/builds/7976045/source?filename=knimin%2Fhandlers%2Fag_new_kit.py you can see my problem.

@wasade
Copy link
Member Author

wasade commented Sep 20, 2016

@jairideout or @gregcaporaso, I think coveralls is acting odd here because of ownership under biocore. I have full admin here but I can't find a way to "authorize" this repo. I added myself to coveralls but I don't have the ability to twerk biocore repos. Any ideas here?

@gregcaporaso
Copy link

gregcaporaso commented Sep 20, 2016 via email

@wasade
Copy link
Member Author

wasade commented Sep 20, 2016

The main page is, but file detail isn't, can you try:

https://coveralls.io/builds/7976045/source?filename=knimin%2Fhandlers%2Fag_new_kit.py

@sjanssen2
Copy link
Collaborator

Do we know how a file must look like for http://localhost:8889/ag_third_party/data/ ?

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

@sjanssen2, I really don't know. @jwdebelius, do you know by chance (I believe this is raw vioscreen data)?

@gregcaporaso
Copy link

Some of the file detail pages seem to be working:
https://coveralls.io/builds/7977612/source?filename=knimin%2Flib%2Fdata_access.py

I see that the one you linked above doesn't have file detail information. If you can see the file detail on this page I doubt this is a permission issue, but let me know if there is anything you want me to try.

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

That one doesn't work for me... see the attached, or is that what you get as well?
screen shot 2016-09-21 at 9 25 13 am

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

Thanks @gregcaporaso btw

@gregcaporaso
Copy link

No, I see the following:

screenshot 2016-09-21 09 26 13

@jairideout is going to change your credentials - I don't think I have access to do that either (or I just don't know where to go to do it). Give us a few mins.

@jairideout
Copy link
Member

@wasade, are you signed into the Coveralls website? I couldn't view those files until I logged in with my GitHub account.

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

Great, thank you!

On Wed, Sep 21, 2016 at 9:28 AM, Greg Caporaso notifications@github.com
wrote:

No, I see the following:

[image: screenshot 2016-09-21 09 26 13]
https://cloud.githubusercontent.com/assets/192372/18719764/8f253a76-7fdd-11e6-91cb-b726a268b20b.png

@jairideout https://github.com/jairideout is going to change your
credentials - I don't think I have access to do that either (or I just
don't know where to go to do it). Give us a few mins.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAc8snqdDjHxVuoOn3ka5Hn9XS0KuOzDks5qsVscgaJpZM4JrbSH
.

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

When I sign in, I get a notice that the source isn't available...

On Wed, Sep 21, 2016 at 9:31 AM, Jai Ram Rideout notifications@github.com
wrote:

@wasade https://github.com/wasade, are you signed into the Coveralls
website? I couldn't view those files until I logged in with my GitHub
account.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAc8siZrrWoZeyU6zx70302l4-oKa8Cyks5qsVvagaJpZM4JrbSH
.

@jairideout
Copy link
Member

Same notice as before, or different message?

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

Different

@jairideout
Copy link
Member

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

yes, @sjanssen2, are you able too as well?

@jairideout
Copy link
Member

Okay, it wasn't a permissions issue (we have the same biocore org permissions). On one of the pages stating the source couldn't be found, there were two text fields that can be configured to help Coveralls find the files based on where they are in the Travis-CI build. The first text field specifies the absolute path to the cloned repo in Travis, and the second specifies a relative path to where the source files are in the repo. I can't remember exactly what I entered for the first text field. I deleted the contents in the second text field. Sorry I can't provide more details; now that the source files are working, Coveralls won't let me navigate to that page with the text fields. If this happens in the future, mess with those text fields until it starts working again. I just fixed the scikit-bio Coveralls source files in a similar manner.

@wasade
Copy link
Member Author

wasade commented Sep 21, 2016

Okay, very weird. Thank you for the investigation and mucking with it :) I
wonder if there was just some hiccup somewhere. It might also be the case
that people don't often check source coverage on individual files regularly
so it might be the case that this has persisted for a bit on a few projects
but just now got noticed

On Wed, Sep 21, 2016 at 9:50 AM, Jai Ram Rideout notifications@github.com
wrote:

Okay, it wasn't a permissions issue (we have the same biocore org
permissions). On one of the pages stating the source couldn't be found,
there were two text fields that can be configured to help Coveralls find
the files based on where they are in the Travis-CI build. The first text
field specifies the absolute path to the cloned repo in Travis, and the
second specifies a relative path to where the source files are in the repo.
I can't remember exactly what I entered for the first text field. I deleted
the contents in the second text field. Sorry I can't provide more details;
now that the source files are working, Coveralls won't let me navigate to
that page with the text fields. If this happens in the future, mess with
those text fields until it starts working again. I just fixed the
scikit-bio Coveralls source files in a similar manner.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAc8smsVQ7bZsJp6HBJNpjpWerZgQ51sks5qsWBpgaJpZM4JrbSH
.

@gregcaporaso
Copy link

Glad it's working, thanks @jairideout!

@jairideout
Copy link
Member

No problem! I suspect it's been an issue for awhile and ppl just don't check the individual files very often. I had to change the text fields several times until it started working, so unlikely to be a hiccup (and a similar fix to scikit-bio resolved the issue). Glad it's working now!

@sjanssen2
Copy link
Collaborator

Now that I am logged in to coverall, I can see the individual file coverage. Thanks for fixing it!

@sjanssen2
Copy link
Collaborator

@mortonjt found that there are already tests for test_ag_stats.py and test_third_party.py. I used test_ag_stats.py and made it more dynamic in an earlier PR. I updated and renamed test_third_party.py in the outstanding PR #118

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

No branches or pull requests

5 participants