Skip to content

integration and regression spec for Java ArrayList merge #7

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

colinsurprenant
Copy link
Contributor

This is a regression spec for elastic/logstash#2372

This spec does not pass on version < 0.1.4 and is now fixed in 0.1.4

@jordansissel
Copy link
Contributor

☑️ Code review
I haven't run the tests yet.

@jsvd
Copy link
Member

jsvd commented Feb 5, 2015

Running the tests I get:

Run options: exclude {:redis=>true, :socket=>true, :performance=>true, :couchdb=>true, :elasticsearch=>true, :elasticsearch_secure=>true, :broken=>true, :export_cypher=>true, :integration=>true}
.F.........

Failures:

  1) LogStash::Filters::Multiline integrations should merge messages arrays as Java ArrayList from json codec using JrJackson
     Failure/Error: results += filter.flush(:final => true)
     ArgumentError:
       wrong number of arguments (1 for 3)
     # ./lib/logstash/filters/multiline.rb:271:in `merge'
     # ./lib/logstash/filters/multiline.rb:270:in `merge'
     # ./lib/logstash/filters/multiline.rb:204:in `flush'
     # ./lib/logstash/filters/multiline.rb:202:in `flush'
     # ./spec/filters/multiline_spec.rb:299:in `(root)'

Finished in 1.42 seconds
11 examples, 1 failure

Failed examples:

rspec ./spec/filters/multiline_spec.rb:251 # LogStash::Filters::Multiline integrations should merge messages arrays as Java ArrayList from json codec using JrJackson

Randomized with seed 44754

@colinsurprenant
Copy link
Contributor Author

@jsvd could you re test please?

@jsvd
Copy link
Member

jsvd commented Mar 16, 2015

Still getting the same error, that comes from within this cycle:

data = events.inject({}) do |result, event|
  self.class.event_hash_merge!(result, event.to_hash_with_metadata, dups_key)
end

@jordansissel jordansissel removed the O(1) label Jun 29, 2015
@purbon
Copy link

purbon commented Sep 4, 2015

@colinsurprenant is this PR still valid? can we fix the test somehow as @jsvd proposed?

@talevy
Copy link

talevy commented Apr 19, 2016

@colinsurprenant @jsvd friendly ping to see what the status on this PR is

@colinsurprenant
Copy link
Contributor Author

ok, I'll reassess the need for this PR - it's been lingering for so long.

@jsvd jsvd removed their assignment Nov 6, 2017
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