Conversation
add ProposalAsserter to make assertions more convenient move CodecNameProposerTest and RecordComponentNameProposerTest to stand-alone test classes
add MethodEntry factory that takes return and param descriptors move SimpleTypeFieldNameProposerTest to stane-alone test class
…nd Identified test inputs, respectively
|
before reviewing, this is very needed. im excited |
OroArmor
left a comment
There was a problem hiding this comment.
I think that every test should be boxed from the others. picking and choosing classes, especially with wildcards just kicks the can further down the road. I think this could be done either with source sets (best option, but idk if you can do something like testInputs/conflict or something. I think the more realistic option is to have com/example/<test-package> and make each of those its own jar. While this would probably lead to duplication among the test classes, i dont think that is that big of a deal, and it means changing one suite doesnt indirectly affect others.
| import static org.quiltmc.enigma_plugin.util.TestUtil.javaLangDescOf; | ||
| import static org.quiltmc.enigma_plugin.util.TestUtil.typeDescOf; | ||
|
|
||
| public interface CommonDescriptors { |
There was a problem hiding this comment.
Not sure how i feel about this. its not bad, but it feels a bit weird in the actual tests.
There was a problem hiding this comment.
Can you elaborate?
Is the issue just with CommonDescriptors, or with the way it's used with the reworked methodOf method?
There was a problem hiding this comment.
imo seeing D or J just feels slightly off. I like the concept but one character values are usually weird and I think im just picking up on that. I'm mostly curious on what others think.
There was a problem hiding this comment.
i like this the way it is, anyone interacting with this code should be familiar enough with bytecode to know the descriptors by heart
I wanted to minimize changes to actual tests for this PR; if we're ok with copy-pasting (maybe renaming) test input classes, I can make it so there's no overlap (it might make Note that |
move test enigma profile to test_enigma tests not yet updated
df3e693 to
9da462a
Compare
…om default enigma profile
f2e9872 to
c70d1a4
Compare
61bc484 to
7a010b0
Compare
7e47577 to
5eedf76
Compare
|
OK, individual test input source sets are working and One slightly annoying thing about the separate source sets is |
ix0rai
left a comment
There was a problem hiding this comment.
i love this. i have Opinions about camel case for folder names but it brings a tear to my eye how beautifully segmented the tests are. great work!
Closes #25
Merging will be a little awkward.
I'll need to update this PR for any tests added by PRs merged before it, and any current PRs merged after it will need to update their tests.
New PRs that add tests should be based on this one.
CommonDescriptorsclass used throughout proposer testsMethodEntryfactory so that it combines return and param descriptors rather than taking a whole method descriptor