eo icon indicating copy to clipboard operation
eo copied to clipboard

#942 tried to convert 0.0.as-bytes to -0.0.as-bytes

Open maxonfjvipon opened this issue 1 year ago • 16 comments

Closes: #942

maxonfjvipon avatar Jul 29 '22 13:07 maxonfjvipon

@Graur maybe it's better to use assert-that here?

yegor256 avatar Jul 29 '22 13:07 yegor256

@yegor256 it'll be assert-that here, ofc, it's just a draft. I'm trying to understand now why map and multimap tests fail

maxonfjvipon avatar Jul 29 '22 13:07 maxonfjvipon

@maxonfjvipon to me the code looks correct. but I may be missing something

yegor256 avatar Jul 29 '22 13:07 yegor256

@maxonfjvipon to me either. But image

maxonfjvipon avatar Jul 29 '22 13:07 maxonfjvipon

@yegor256 @Graur ideas or guesses are welcome :)

maxonfjvipon avatar Jul 29 '22 13:07 maxonfjvipon

@maxonfjvipon can you show the full stacktrace you get from this unit test?

yegor256 avatar Jul 29 '22 14:07 yegor256

@yegor256 [INFO] Running EOorg.EOeolang.EOcollections.EOmap_of_mapsTest Error: Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.722 s <<< FAILURE! - in EOorg.EOeolang.EOcollections.EOmap_of_mapsTest Error: EOorg.EOeolang.EOcollections.EOmap_of_mapsTest.testWorks Time elapsed: 0.718 s <<< ERROR! EOorg.EOeolang.EOerror$ExError: EOorg.EOeolang.EOstringν108782="The argument '.x' is of Java type 'java.lang.Double', not 'java.lang.Long' as expected"SF at org.eolang.PhSafe.attr(PhSafe.java:97) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhSafe.attr(PhSafe.java:94) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhSafe.attr(PhSafe.java:94) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhImmovable.attr(PhImmovable.java:85) at org.eolang.PhLocated.attr(PhLocated.java:99) at org.eolang.PhOnce.attr(PhOnce.java:96) at org.eolang.PhOnce.attr(PhOnce.java:96) at org.eolang.Dataized.take(Dataized.java:80) at EOorg.EOeolang.EObool$EOor.lambda$new$0(EObool$EOor.java:67) at org.eolang.AtComposite.get(AtComposite.java:73) at org.eolang.CachedPhi.get(CachedPhi.java:81) at org.eolang.PhDefault.attr(PhDefault.java:241) at org.eolang.PhLocated.attr(PhLocated.java:99) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhSafe.attr(PhSafe.java:94) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhSafe.attr(PhSafe.java:94) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) at org.eolang.PhNamed.attr(PhNamed.java:92) ....

UPD: for some reason these tests don't fail on my local machine

maxonfjvipon avatar Jul 29 '22 14:07 maxonfjvipon

@maxonfjvipon I think the main problem is when the key of the map is 0. Try this short example:

[] > map-with-zero-key
  (map *).with 0 2 > m
  assert-that > @
    (m.found 0).at 0
    $.equal-to 2

it should fail.

Graur avatar Jul 29 '22 15:07 Graur

@Graur id doesn't and I think it shouldn't. There is no any special about this zero-key. Your test also works fine. Screen Shot 2022-07-29 at 10 01 39 PM image

maxonfjvipon avatar Jul 29 '22 19:07 maxonfjvipon

Codecov Report

Merging #957 (8dbbb81) into master (36f1b85) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #957      +/-   ##
============================================
+ Coverage     79.03%   79.06%   +0.02%     
- Complexity      584      585       +1     
============================================
  Files           148      148              
  Lines          3024     3028       +4     
  Branches        306      307       +1     
============================================
+ Hits           2390     2394       +4     
  Misses          528      528              
  Partials        106      106              
Impacted Files Coverage Δ
...e/src/main/java/EOorg/EOeolang/EObytes$EOleft.java 100.00% <100.00%> (ø)
.../src/main/java/EOorg/EOeolang/EObytes$EOright.java 100.00% <100.00%> (ø)
...ntime/src/main/java/EOorg/EOeolang/EOint$EOgt.java 100.00% <100.00%> (ø)
eo-runtime/src/main/java/org/eolang/Param.java 66.07% <100.00%> (+2.60%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

codecov-commenter avatar Jul 29 '22 19:07 codecov-commenter

@yegor256 @Graur is there a way I can see what object has this attribute/argument x of wrong type? Mb somewhere in xmir? image

maxonfjvipon avatar Jul 30 '22 09:07 maxonfjvipon

@Graur @yegor256 so, I found out that the exception is thrown is strong method of Param class when the argument of the method is Long.class. I found that there are 3 places where this method is called with Long.class argument and the name of the attribute is x. Only in one place exception is thrown - EOint$EOgt.

I put changes into strong method image

I found that the argument we try compare int to is 0.0. So some-int.gt 0.0 and here exception is thrown.

Here's phi term: image

Here is our -1 from map-of-maps test.

...
eq. > @
    list
      * 0 "d"
    * ((mp-res.found -1).length) ((((mp-res.found 1).at 0).found "c").at 0)

-1.as-hash is 0. There are not so many places where 0.eq 0.0 is possible - all of them is number. Please not that we haven't merged this one yet. It means that there is a mistake in number object that eo-runtime loads on tests. image

map and multimap in their found methods call number.mod object. number.mod calls number.abs and in number.abs we have: image

-1.as-hash is 0. 0.is-int should be TRUE, but it's FALSE now because of mistake in number. So then we go to: 0.gte 0.0 and here's our exception.

I'm not sure I'm right and I don't know why these test work fine before but it seems we should merge this one, add it to objectionary and then try here again.

maxonfjvipon avatar Jul 30 '22 13:07 maxonfjvipon

@maxonfjvipon Now all checks should pass, please try it

Graur avatar Aug 11 '22 14:08 Graur

@Graur something's wrong with maven-plugin tests

maxonfjvipon avatar Aug 11 '22 15:08 maxonfjvipon

@maxonfjvipon Indeed, it was another checkstyle problem. Please, try it one more time

Graur avatar Aug 11 '22 18:08 Graur

@Graur @yegor256 Hallelujah! I think it's good to merge

maxonfjvipon avatar Aug 11 '22 18:08 maxonfjvipon

@Graur I hope now it's good to merge

maxonfjvipon avatar Aug 12 '22 11:08 maxonfjvipon

@yegor256 Could you check these changes, please?

Graur avatar Aug 15 '22 14:08 Graur

@rultor merge

yegor256 avatar Aug 15 '22 14:08 yegor256

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor avatar Aug 15 '22 14:08 rultor

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 7min)

rultor avatar Aug 15 '22 14:08 rultor