Skip to content

Conversation

@inpink
Copy link

@inpink inpink commented Nov 6, 2025

[Please describe here what your change is about]

  • Added a dropTestData() attribute to @SessionFactory (default NEVER) so tests can opt into automatic cleanup declaratively.
  • Introduced the DropDataTiming enum to capture supported cleanup moments (NEVER, BEFORE/AFTER_EACH, BEFORE/AFTER_ALL).
  • Updated SessionFactoryExtension to store the configured timing and invoke scope.dropData() during the matching JUnit lifecycle callbacks (before/after each or all tests).
  • Added DropDataTimingTest to prove each timing option behaves as expected by inserting data and asserting whether it persists or gets cleared at the right moments.
  • Added SessionFactoryProducerDropDataTest to show that tests using only SessionFactoryProducer (no annotation) default to DropDataTiming.NEVER, keeping data until explicitly dropped.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19912

@inpink inpink marked this pull request as draft November 6, 2025 18:25
@inpink inpink force-pushed the inpink/HHH-19912 branch 2 times, most recently from ea9ba16 to 374fdb8 Compare November 6, 2025 18:31
@inpink inpink force-pushed the inpink/HHH-19912 branch 3 times, most recently from 786f305 to ad5c292 Compare November 9, 2025 12:53
@inpink inpink marked this pull request as ready for review November 9, 2025 12:54
@inpink inpink requested a review from sebersole November 9, 2025 12:54
@sebersole
Copy link
Member

I still wonder if we want to allow multiple options here (array attribute). Its hidden in the Jira a bit, but I mean specifically this thought -

DropDataTiming[] dropTestData() default {}

What do you think?

@inpink
Copy link
Author

inpink commented Nov 13, 2025

@sebersole Good idea! I think using BEFORE_EACH and AFTER_EACH together doesn't provide significant benefits. However, combining BEFORE_ALL or AFTER_ALL with other timing options would be quite effective. If you agree, I'll modify the implementation to support multiple timing values as an array.

If using an array, we would need to handle cases where NEVER is used together with other timings. How about throwing an exception like ValidationMode does?

@sebersole
Copy link
Member

sebersole commented Nov 13, 2025

You can just get rid of NEVER in that case. NEVER is effectively {}

@inpink
Copy link
Author

inpink commented Nov 13, 2025

@sebersole "Thanks for the feedback! Just to confirm my understanding - when NEVER is used with other timing values like {NEVER, BEFORE_ALL}, you're suggesting to simply remove NEVER and execute only {BEFORE_ALL}, correct?

@sebersole
Copy link
Member

No, not quite. I'm saying remove NEVER as an enum value.

Given the definition:

DropDataTiming[] dropTestData() default {}

The default is exactly the same as "never".

@inpink
Copy link
Author

inpink commented Nov 13, 2025

@sebersole Ah, I understand now! I'll modify the implementation to remove the NEVER enum value and update the logic accordingly.

@sebersole
Copy link
Member

More concretely...

public enum DropDataTiming {
    BEFORE_EACH,
    AFTER_EACH,
    BEFORE_ALL,
    AFTER_ALL
}

@interface SessionFactory {
    ...
    DropDataTiming[] dropTestData() default {}
}

Also, I'd consider adding :

interface SessionFactoryProducer {
    static final DropDataTiming[] NONE = new DropDataTiming[0];

    default DropDataTiming[] dropTestData() {
        return NONE;
    }
}

@inpink
Copy link
Author

inpink commented Nov 13, 2025

@sebersole Thanks for the feedback! I've implemented the default {} approach.
Regarding the second code block, let me think about it a bit more. It's nighttime in Korea now. Haha. My understanding is that you're suggesting to support dropData functionality for cases where SessionFactoryProducer is implemented as well - is that correct?

@sebersole
Copy link
Member

My understanding is that you're suggesting to support dropData functionality for cases where SessionFactoryProducer is implemented

Correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants