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

Adding tests for complete coverage of Polynomial #893

Merged
merged 5 commits into from Apr 10, 2016

Conversation

Projects
None yet
2 participants
@CodeMaxx
Copy link
Contributor

CodeMaxx commented Mar 30, 2016

I'll keep amending the commit after adding more tests
Please comment if you think more tests are required in some place.

@isuruf @Sumith1896

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:coverage branch 4 times, most recently from 017182e to 887997a Mar 30, 2016

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:coverage branch from 887997a to d496b85 Apr 1, 2016

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:coverage branch from d496b85 to ae6e2cb Apr 2, 2016

@CodeMaxx CodeMaxx changed the title [WIP] Adding tests for complete coverage Adding tests for complete coverage Apr 2, 2016

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:coverage branch from ae6e2cb to 699d3ce Apr 2, 2016

@CodeMaxx CodeMaxx changed the title Adding tests for complete coverage Adding tests for complete coverage of Polynomial Apr 3, 2016

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Apr 8, 2016

@isuruf Ping

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Apr 9, 2016

Tests are failing

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Apr 9, 2016

@isuruf Thats a problem with Travis.... the tests are passing otherwise.

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Apr 9, 2016

I restarted Travis. Appveyor failures seems legit. Can you look into it?

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Apr 9, 2016

@isuruf I'm not able to understand the error.

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Apr 9, 2016

See here, https://ci.appveyor.com/project/certik/symengine/build/1418/job/cw70oi8hp6hjmru2#L1760
It's complaining that << is not defined for RCP<const Symbol> which is true


REQUIRE(a->max_coef() == 4);
REQUIRE(not(a->max_coef() == 2));
REQUIRE(b->max_coef() == symbol("b"));

This comment has been minimized.

@isuruf

isuruf Apr 9, 2016

Member

Use eq here.

Error is due to Catch uses macros to take the LHS and RHS separately and will try to print each side with << to show that RHS and LHS are different if there are different.

This comment has been minimized.

@CodeMaxx

CodeMaxx Apr 9, 2016

Contributor

Actually max_coeff() returns Expression so I'll change symbol to Expression and use ==

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Apr 9, 2016

@isuruf Working now

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Apr 9, 2016

@isuruf Ping

@isuruf isuruf merged commit 54d1e1f into symengine:master Apr 10, 2016

3 checks passed

codecov/project 75.71% (+0.78%) compared to 70e0188 at 74.93%
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Apr 10, 2016

Coverage was improved by 0.78%. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment