yast / yast-storage-ng

Rewrite of https://github.com/yast/yast-storage
http://yast.github.io/
GNU General Public License v2.0
14 stars 20 forks source link

Ruby 3.2 yast2-storage-ng sometimes fails to build, diff in Marshal.dump #1321

Closed mvidner closed 1 year ago

mvidner commented 1 year ago

Problem

Ruby 3.2 yast2-storage-ng sometimes fails to build, diff in Marshal.dump

The failing test means to check that a new ProposalSettings is in fact equal to the original one, and does so by dumping and reloading the two objects with Marshal. It fails in case the two objects differ in the order in which their instance variables were assigned.

Solution

Instead of using Marshal, add ProposalSettings#==.

Do it by introducing a simple module, EqualByInstanceVariables, including it in that class and a couple of others that are used by it.

Gory details: everything I've learned about equality in Ruby along the way, very messy so far: https://gist.github.com/mvidner/eb10bf4d70f0df5c1073a2229fd4491e

Testing

Screenshots

N/A

mvidner commented 1 year ago

Today's findings:

The Marshal dumps are different because some otherwise equal objects objects have different ordering of instance variables, namely in VolumeSpecification objects. Why, yes, I did spend a stupid amount of time to find the reason for this difference. Unsuccessfully.

My plan is to accept the reality of the nondeterministic ordering and instead remove the gorilla from the room: Marshal. We are using it because ProposalSettings#== does not work properly

WIP:

irb(main):001:0> class C; def initialize(attrs); attrs.each { |a| instance_variable_set(a.to_sym, a.to_s) } ; end ; end
=> :initialize
irb(main):004:0> a = C.new([])
=> #<C:0x0000563aa62d27b0>
irb(main):005:0> b = C.new([])
=> #<C:0x0000563aa618b848>
irb(main):006:0> a == b
=> false
irb(main):007:0> module EqualByIVs; def ==(other); return if other.class != self.class; instance_variables.all? { |iv| instance_variable_get(iv) == other.instance_variable_get(iv) }; end; end
=> :==
irb(main):008:0> class C; include EqualByIVs; end
=> C
irb(main):009:0> a == b
=> true
coveralls commented 1 year ago

Coverage Status

Coverage: 97.755% (+0.001%) from 97.754% when pulling 567058c3bad1dbd84707a4646d36e961b0150593 on ruby32-proposal-settings-comparison into 7b600199d83bd173d5e524aba362d21de11c66f8 on master.

yast-bot commented 1 year ago

:heavy_check_mark: Public Jenkins job #452 successfully finished :heavy_check_mark: Created OBS submit request #1069105

yast-bot commented 1 year ago

:heavy_check_mark: Internal Jenkins job #1138 successfully finished :heavy_check_mark: Created IBS submit request #291247