[openstreetmap/openstreetmap-website] Problems with the way#test_max_nodes_per_way_limit (#1516)

Andy Allan notifications at github.com
Wed Apr 5 11:05:50 UTC 2017


The [way#test_max_nodes_per_way_limit](https://github.com/openstreetmap/openstreetmap-website/blob/e4e5ec44099eb3f21ee691058688cdd4e5d25e50/test/models/way_test.rb#L27-L41) doesn't work. It looks like it keeps adding nodes until it reaches the limit, tests everything is fine, then adds on more and tests again. 

Unfortunately the final test shows that the `valid?` is still true. So it's not really testing the limit at all. That was probably meant to be a final `assert !way.valid?`

The root cause is that [the number of way_nodes isn't tested in any of the validations](https://github.com/openstreetmap/openstreetmap-website/blob/e4e5ec44099eb3f21ee691058688cdd4e5d25e50/app/models/way.rb#L22-L30), so the way is always valid no matter how many nodes are added. Instead we have a `preconditions_ok?` check that is called in some code paths (e.g. `save_with_history!` but, notably, not `save`)

Should we:
1) Update the test to use the `preconditions_ok?` instead of `valid?`
or
2) Change the model so that the code in `preconditions_ok?` is used for all save methods (`save`, `save!` etc) and validity tests (i.e. `valid?`) via normal rails validations?
or
3) Something else?


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/1516
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20170405/36a0da6f/attachment.html>


More information about the rails-dev mailing list