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

test improvements #50

Merged
merged 7 commits into from
Jun 12, 2015

Conversation

Maks3w
Copy link
Member

@Maks3w Maks3w commented Jun 8, 2015

No description provided.

@Maks3w Maks3w added this to the 1.0.3 milestone Jun 8, 2015
This test method asserts trait customer constructor which is totally unrelated with the purpose of the whole test case
$headers = ['X-Foo' => ['bar']];
$this->message = new Request(null, null, $this->stream, $headers);
$this->assertSame($headers, $this->message->getHeaders());
}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this test?

@weierophinney weierophinney modified the milestones: 1.0.4, 1.0.3 Jun 12, 2015
@weierophinney weierophinney self-assigned this Jun 12, 2015
@weierophinney weierophinney merged commit fefde14 into zendframework:master Jun 12, 2015
weierophinney added a commit that referenced this pull request Jun 12, 2015
weierophinney added a commit that referenced this pull request Jun 12, 2015
- Reinstated the two tests that were removed.
- Updated property docblocks to be full length vs single-line, per our CS.
weierophinney added a commit that referenced this pull request Jun 12, 2015
weierophinney added a commit that referenced this pull request Jun 12, 2015
@weierophinney
Copy link
Member

I re-instated the two tests that were removed when merging.

@Maks3w
Copy link
Member Author

Maks3w commented Jun 12, 2015

@weierophinney Those tests was not testing the trait. Those tests was testing Request constructor

It's totally out of the scope

@weierophinney
Copy link
Member

Then moving them to the correct test suite would have worked, but not
removing them entirely. Send a new PR if you wish to do that.
On Jun 12, 2015 1:20 PM, "Maks3w" notifications@github.com wrote:

@weierophinney https://github.com/weierophinney Those tests was not
testing the trait. Those tests was testing Request constructor

It's totally out of the scope


Reply to this email directly or view it on GitHub
#50 (comment)
.

@Maks3w
Copy link
Member Author

Maks3w commented Jun 12, 2015

@weierophinney Please I can't fix all the test suite at once. I need to go by steps

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

Successfully merging this pull request may close these issues.

2 participants