Skip to content

Conversation

@electrical
Copy link

First step of the Rspec refactoring for #1737

@elasticsearch-release
Copy link

🔴 Test failed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/114/

@elasticsearch-release
Copy link

🔴 Test failed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/115/

@elasticsearch-release
Copy link

🔴 Test failed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/116/

@elasticsearch-release
Copy link

🔴 Test failed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/117/

@elasticsearch-release
Copy link

🔴 Test failed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/118/

@elasticsearch-release
Copy link

💚 Test passed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/119/

Copy link
Contributor

Choose a reason for hiding this comment

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

Yessss!

@jordansissel
Copy link
Contributor

Did a quick review. Overall LGTM. I will test.

@elasticsearch-release
Copy link

💚 Test passed.
Refer to this link for build results: http://build-eu-1.elasticsearch.org/job/Logstash_PR/121/

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason these regression tests were removed?

Copy link
Author

Choose a reason for hiding this comment

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

All tests were either duplicate ( already present in the actual spec file ) or i've merged them together in the spec file of the filter it self.

@jordansissel
Copy link
Contributor

LGTM except for my comment about regression test removal

@jordansissel
Copy link
Contributor

LGTM.

@jordansissel
Copy link
Contributor

@electrical can you rebase this against master?

Richard Pijnenburg added 7 commits September 30, 2014 09:29
- Move helper functions in own modules and extend Rspec
- Refactor files into correct naming and paths
- Modify files to use new spec_helper and helpers
- Pin rspec to 2.14.x
- date speed test only run when performance is enabled
When doing randomized testing, one of the tests failed because kafka was not initialized yet
@jordansissel
Copy link
Contributor

(break == many PRs have tests, I think this may break merges, but it's the right change so we'll just have to step up some PR help to compensate)

@electrical
Copy link
Author

Allot of PR's will need to be directed to the individual repo's anyway when im done with that ( soon )

@jordansissel
Copy link
Contributor

+1

@wiibaa
Copy link
Contributor

wiibaa commented Sep 30, 2014

@jordansissel talking about the split to individual repos, will you share a magic github-octokit command to easily watch them all ??

@jordansissel
Copy link
Contributor

@wiibaa yeah we'll publish something to do that.

electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
- date speed test only run when performance is enabled

Fixes #1758
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
When doing randomized testing, one of the tests failed because kafka was not initialized yet

Fixes #1758
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
- Move helper functions in own modules and extend Rspec
- Refactor files into correct naming and paths
- Modify files to use new spec_helper and helpers
- Pin rspec to 2.14.x

Fixes #1758
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
- date speed test only run when performance is enabled

Fixes #1758
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
When doing randomized testing, one of the tests failed because kafka was not initialized yet

Fixes #1758
electrical pushed a commit that referenced this pull request Sep 30, 2014
electrical pushed a commit that referenced this pull request Sep 30, 2014
@colinsurprenant
Copy link
Contributor

post-merge LGTM ;) Good job!

@jordansissel jordansissel added this to the v1.5.0 milestone Nov 11, 2014
@electrical electrical deleted the refactor/rspec branch December 16, 2014 15:41
@electrical electrical removed their assignment Feb 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants