Skip to content

Conversation

@mabdi
Copy link
Contributor

@mabdi mabdi commented Aug 21, 2020

I submit this pull request to suggest a test method PMVectorTest >> testHouseholder.

We noticed that the method #householder is never executed by any of the tests in PMVectorTest. Therefore we created a test method which uses two vectors to exercise both branches in line 203 (x <=0) ifTrue: [x -u] ifFalse: [0 - s / (x + u)]. In the former vector x = -1 forces the ifTrue branch in the latter vector x = 1.00001 forces the ifFalse branch.

Note that these suggestions are adapted from a test amplification tool called SmallAmp (https://github.com/mabdi/small-amp). SmallAmp executes existing tests, sees which parts of the class under test are not covered and then suggests improvements on the test methods.

I hope you will accept this pull request. It would illustrate that SmallAmp makes relevant suggestions.

Mehrdad Abdi.

u := #(-1 0 1) asPMVector. "`x <= 0` when x = -1"
w := u householder.
self
assert: w class equals: Array;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can omit this check.

u := #(1.00001 2.00007) asPMVector. "`x <= 0` when x = 1.00001"
w := u householder.
self
assert: w class equals: Array;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can omit this, too, I think.

@SergeStinckwich
Copy link
Member

Thank you @mabdi for your contribution ! Can you take care about the remarks of Hemal and we can integrate your test.

@mabdi
Copy link
Contributor Author

mabdi commented Aug 24, 2020

Of course, I omitted the type-assertions. Please check again.

@SergeStinckwich SergeStinckwich merged commit 87094ea into PolyMathOrg:master Aug 24, 2020
@SergeStinckwich
Copy link
Member

Thank you for increasing our test coverage ;-)

@SergeStinckwich SergeStinckwich added this to the v1.0.3 milestone Feb 11, 2021
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.

3 participants