Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 18, 2017

We had a TODO about adding tests around cached boxing. In #24077
I tracked down the uncached boxing tests and saw the TODO. Cached
boxing testing is a fairly small extension to that work.

We had a TODO about adding tests around cached boxing. In elastic#24077
I tracked down the uncached boxing tests and saw the TODO. Cached
boxing testing is a fairly small extension to that work.
@nik9000 nik9000 added :Plugin Lang Painless >test Issues or PRs that are addressing/adding tests v5.5.0 v6.0.0-alpha1 labels Apr 18, 2017
@nik9000 nik9000 requested a review from jdconrad April 18, 2017 17:02
/* Now check that we use valueOf with the boxing used for comparing primitives to def. For this we need an
* integer that is cached by Integer.valueOf. The JLS says 0 should always be cached. */
int cachedAutoboxedInt = 0;
assertEquals(Integer.valueOf(cachedAutoboxedInt), Integer.valueOf(cachedAutoboxedInt));
Copy link
Member

Choose a reason for hiding this comment

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

I took a look because it looked fun, sorry if I'm getting things wrong. If I understand this test right, then these two values should be referentialy equal, so shouldn't this be assertSame?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably!

/* Now check that we use valueOf with the boxing used for comparing primitives to def. For this we need an
* integer that is cached by Integer.valueOf. The JLS says 0 should always be cached. */
int cachedAutoboxedInt = 0;
assertEquals(Integer.valueOf(cachedAutoboxedInt), Integer.valueOf(cachedAutoboxedInt));
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@cbuescher
Copy link
Member

@nik9000 @jdconrad I'm not too familiar with painless to approve this, but I thought since I took a look already I might a as well leave some questions.

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM. My apologies, not sure why I never got back to this.

@nik9000 nik9000 merged commit 4a06dd9 into elastic:master Oct 10, 2017
@nik9000 nik9000 added v7.0.0 and removed v6.0.0 labels Oct 10, 2017
nik9000 added a commit that referenced this pull request Oct 10, 2017
We had a TODO about adding tests around cached boxing. In #24077
I tracked down the uncached boxing tests and saw the TODO. Cached
boxing testing is a fairly small extension to that work.
jasontedor added a commit that referenced this pull request Oct 12, 2017
* master: (35 commits)
  Create weights lazily in filter and filters aggregation (#26983)
  Use a dedicated ThreadGroup in rest sniffer (#26897)
  Fire global checkpoint sync under system context
  Update by Query is modified to accept short `script` parameter. (#26841)
  Cat shards bytes (#26952)
  Add support for parsing inline script (#23824) (#26846)
  Change default value to true for transpositions parameter of fuzzy query (#26901)
  Adding unreleased 5.6.4 version number to Version.java
  Rename TCPTransportTests to TcpTransportTests (#26954)
  Fix NPE for /_cat/indices when no primary shard (#26953)
  [DOCS] Fixed indentation of the definition list.
  Fix formatting in channel close test
  Check for closed connection while opening
  Clarify systemd overrides
  [DOCS] Plugin Installation for Windows (#21671)
  Painless: add tests for cached boxing (#24163)
  Don't detect source's XContentType in DocumentParser.parseDocument() (#26880)
  Fix handling of paths containing parentheses
  Allow only a fixed-size receive predictor (#26165)
  Add Homebrew instructions to getting started
  ...
jasontedor added a commit that referenced this pull request Oct 12, 2017
* 6.x: (32 commits)
  Use a dedicated ThreadGroup in rest sniffer (#26897)
  Fire global checkpoint sync under system context
  Update by Query is modified to accept short `script` parameter. (#26841)
  Cat shards bytes (#26952)
  Adding unreleased 5.6.4 version number to Version.java
  Rename TCPTransportTests to TcpTransportTests (#26954)
  Fix NPE for /_cat/indices when no primary shard (#26953)
  [DOCS] Fixed indentation of the definition list.
  Check for closed connection while opening
  Clarify systemd overrides
  [DOCS] Plugin Installation for Windows (#21671)
  Painless: add tests for cached boxing (#24163)
  Don't detect source's XContentType in DocumentParser.parseDocument() (#26880)
  Return List instead of an array from settings (#26903)
  Emit deprecation warning for variable size predictor
  Fix handling of paths containing parentheses
  Deprecate variable-size receive predictor
  Add Homebrew instructions to getting started
  ingest: Fix bug that prevent date_index_name processor from accepting timestamps specified as a json number
  Scripting: Fix expressions to temporarily support filter scripts (#26824)
  ...
@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >test Issues or PRs that are addressing/adding tests v6.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants