Closed lynaghk closed 6 years ago
@pkriens It seems like you rebased some commits on this branch after I forked and opened this pull request. I'll rebase my own commits on top of your latest so the diff is clearer.
Edit: On second thought, it looks like as part of your rebase you wrote similar tests (e.g., https://github.com/AlloyTools/org.alloytools.alloy/commit/8a6c478bcba99e81918f25cafe0b5a6ba725c58b#diff-87e1d06259458ba3f6ce21237bf06c3dR53) as my commits.
I'll wait to hear back from you, @pkriens, before cherry picking my commits or otherwise changing anything. Please let me know how you'd like me to proceed on this.
The problem is fixed in the pkriens/api branch because I wanted to do this slightly different, but this PR clearly helped. Thanks.
Hi @pkriens, this pull request follows up on our email conversation. There are two commits/changes:
Add a test to ensure that instances returned from the solutions iterator are not coupled to the iterator state (as they accidentally were before a39b993). I'm not sure if there's an idiomatic way to compare tuple sets by value, so the test compares the
toString
representations --- would love feedback on this if there's a proper way to do it w/ the existing methods.Fix the solutions iterator to return the correct number of solutions (that is, match the Alloy GUI).
This is also my first pull request, so a few other notes in case I'm doing something wrong:
I ran the tests via:
./gradlew --info org.alloytools.alloy.classic.test:test
I edited the source with IntelliJ. Hopefully that's smart enough to match the existing formatting. I don't have Eclipse installed and so couldn't do the autoformatting alluded to in the contribution guidelines. I'm on OS X and would be happy to install run some kind of command-line formatter if there are specific instructions or a shell script somewhere.