Last minute rush: debug failing tests

[enewe101]

Hey @asutcl,

Most of the merges seemed to go well, but there are a couple issues left. Right now in integration we have several errors and failures in tests. I've walked all over the history tree testing and am fairly confident that I've traced this to a specific commit

commit 4eca904c1ff91114b0f5d53a1b2b359e68386cb9
Author: Andrew Sutcliffe <sutcliffea@gmail.com>
Date:   Wed Mar 19 16:23:46 2014 -0400

    ScoredAttribute now has a correlation score

This is one of your commits. I'm trying to diagnose the problem but it's getting late, and I'm not sure what is wrong, so, if you're available, could you have a look?

This commit is from Wednesday, so it is somewhat strange that it is showing up. I found this using git bisect which takes a fairly principled approach to narrowing in on the problem, and I've searched all around manually too.

What I suspect has happened is that git has gotten confused, because there were commits of yours on either side of the merge into integration (ones coming from master and Issue0044) -- so it might have given the wrong files precedent. Remember when we encountered this late after a group meeting before? Could you have a look at the current files vs what you integrated most recently into Issue0044? I'm positive that there was no problem when you integrated into Issue0044 because I ran tests myself.

Meanwhile I'm going to keep trying to figure it out...

[asutcl]

Hi Edward,

yeah some of the pulls where changing my local project files and I was having trouble with that. I was having trouble with that.

The test I found was failing was because of the attribute correlation on non numeric data. Which is weird because I do a check first. Are there any other errors on the tests other than on AttributeExtractor?

[asutcl]

From looking at it quick, the problem I am getting seems to be from the fact that some attribute will mostly be numeric and then one filed will be Data not available for example. This causes the AttributeCorrelator to send an error which I don't catch.

I can catch it for this realase and default the correlatoin to zero but we will need to deal with this later.

[enewe101]

Ok let's try that. I was posting a longer message about other possibilities -- but if this works that's a good plan for now!

[asutcl]

Ok I still get errors on testratingsandspecs and test range in the TypedValue, any ideas for those?

Can I push the fix for AttributeExtractor to Integration?

[asutcl]

Integration branch still has ratings and specs which are seperate in category. Am I working on the right code?

[enewe101]

I'm really starting to suspect that this is coming from the other side of the commit. There is definitely an issue introduced in code that was a little bit upstream of the merge where the problems started:

commit e19443f44f046634c0dfe5ca9c451cfb76cb851a
Author: gowri <mangalagb@gmail.com>
Date:   Fri Mar 21 20:07:40 2014 -0400

    Added range detection in TypedValue

That commit had testRange failing -- @mangalagb maybe you could look into this possibility?

The current errors are:

• testAllCategories
• testratingsandspecs
• testRange (that comes in an earlier commit, the one above)
• testFunction_Initialreturn2

If the owners could look in the integration branch and try to find the problem it would be most appreciated. I'll continue to search as well.

[enewe101]

Just read your comment... I can't believe it! My code got overwritten. K, let me restore that, and figure out where that happened...

[asutcl]

I've had problems with the project git since Tuesday. I don't know what happened but it's been behaving weirdly all week....

[enewe101]

Wait, wait... my code is ok -- it's only in Category, and it's the getters for AttributeStat. Since these are superceded by AttributeExtractor I just left them (I'm phasing it out). If you look in product you'll see no specs / ratings stuff, and everywhere else in the code its all changed to *Attribute.

[enewe101]

Alright -- It's quite late, so I don't think it makes sense to get everyone searching through their code right now. We might have to cut our losses, and track the bug tomorrow morning.

I would like to kindly ask each of the owners of the failing tests to attempt to trace back the cause of their failing tests, then post here.

Also, I think we need to understand whether something was introduced at commit e19443f44 that I cited above.

Let's call it a night :)

[mangalagb]

Something weird is going on. When you pull the latest Integration, all tests run fine except for testratingsandspecs() which gives me an error. Is everyone getting the same error or is it just me?

[nishanthtgwda]

@mangalagb , I have the error as well. I wrote the test cases for testratingsandspecs() , The count for attributes of type ratings was 6 in "Humidifiers" , overall score, output, noise, effeciency, convenience, hard water. Right now, the Data API is returning the count as 27 . And thats why the test cases are producing the fail result. @enewe101 , Did anyone else modify the getRatings() method, cause it has to return 6, atleast for the humidifers ?

[asutcl]

If product ratings and specs have been collapsed into attributes (at the product level), is it not possible that 27 is the count for both? I am not sure of the implementation but if the category builds ratings and specs list from products this may be the case.

[enewe101]

There were some changes this morning to TypedValue. I tracked down the error and then made a change. When I went to commit I got a conflict with someone elses change (couldn't find it in the log though) -- they were nearly identical, so I took there's instead.

@nishanth1991 I didn't touch testratingsandspecs() but getRatings() is not supported anymore. I made changes throughout the code, but I did so on the integration branch -- therefore your test probably didn't get updated. This shouldn't be hard to fix. I'm going to update your test now ok?

[enewe101]

Yes, that was the case. And Andrew is correct.

So, under this implementation, clients of CRData shouldn't access getRatings or getSpecs on categories -- these return AttributeStat's which are superseded by the AttributeExtractor which is what clients should access.

One thing that seems to have been difficult is that it hasn't been obvious which branch people should dev off and integrate to. The result is that errors surface late in the integration. Let's discuss it when gameplan milestone 4. There must be some 'best practice' workflow to avoid stuff like that.

Right now everything is passing. I'm now merging with master...

[enewe101]

We have our milestone 3 release!

[enewe101]

So I'm running through usability tests and...

well, I don't want to be a downer, but this doesn't seem to do what we wanted. Isn't the list of ranked products supposed to update when we check some features and click submit? Did this functionality just get lost in a merge, and if so, can anyone point at a branch where it worked?

[MariamN]

I just pulled from master, and the list is updated after u select some features. Can u tell me what is the test scenario that u used.

[enewe101]

Huh weird. No matter what I try I don't get products updating. Here's one example:

• query "apple"
• choose "tablets"
• select any combination of features and hit submit

For me, the page reloads, but it appears to be the same product list, still alphabetical.

[enewe101]

Ok -- interesting. It works for me on Chrome, but not Firefox.

Looks like this might be a cross-browser thing. That's a relief, because it means that the underlying machinery is working. I do notice that I get unusual browser handling of the form submission in firefox -- it behaves as if I'm reloading a page that had been sent as a reply to a POST (I.e, it asks me to confirm "resending" data, even though I haven't yet posted data, or so it seems).

So let's just chalk it up as an issue for milestone 4?

[enewe101]

@priyasidhaye maybe we should include testing on different browsers in your test-cases.

[priyasidhaye]

Yes, I will update the tests after trying it on different browsers.

On Sat, Mar 22, 2014 at 2:41 PM, enewe101 notifications@github.com wrote:

@priyasidhaye https://github.com/priyasidhaye maybe we should include testing on different browsers in your test-cases.

Reply to this email directly or view it on GitHubhttps://github.com/prmr/Creco/issues/61#issuecomment-38360410 .