Skip to content

Extend default script detection in @Sql to account for execution phase [SPR-13322] #17907

New issue
New issue
Open
Open
Extend default script detection in @Sql to account for execution phase [SPR-13322]#17907
Labels
in: testIssues in the test moduletype: enhancementA general enhancement
Milestone
General Backlog
@spring-projects-issues

Description

@spring-projects-issues
spring-projects-issues
opened on Aug 6, 2015
Collaborator

Robert Thaler opened SPR-13322 and commented

Status Quo

We often need to prepare data before a specific test and clean up this data after the test. Therefore we use two distinct scripts assigned to the test's ExecutionPhase.

Proposal

Appending a suffix such as _before or _after to the default script name would allow one to use default detection with different scripts for preparation and cleanup.

Deliverables

  1. Default script detection should take the ExecutionPhase into account.

Affects: 4.1 GA

Activity

spring-projects-issues
added
status: waiting-for-triageAn issue we've not yet triaged or decided on
in: testIssues in the test module
type: enhancementA general enhancement
and removed
type: enhancementA general enhancement
on Jan 11, 2019
rstoyanchev
added
status: bulk-closedAn outdated, unresolved issue that's closed in bulk as part of a cleaning process
and removed
status: waiting-for-triageAn issue we've not yet triaged or decided on
on Jan 11, 2019
spring-projects-issues

spring-projects-issues commented on Jan 12, 2019

@spring-projects-issues
spring-projects-issues
on Jan 12, 2019
CollaboratorAuthor

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

spring-projects-issues
closed this as completedon Jan 12, 2019
sbrannen
self-assigned this
on Jan 15, 2019
sbrannen
removed their assignment
on Apr 22, 2021
sbrannen
added
type: enhancementA general enhancement
on Apr 22, 2021
sbrannen
mentioned this on Apr 22, 2021
sbrannen
removed
status: bulk-closedAn outdated, unresolved issue that's closed in bulk as part of a cleaning process
on Apr 22, 2021
sbrannen

sbrannen commented on Apr 22, 2021

@sbrannen
sbrannen
on Apr 22, 2021
Member

Reopening due to renewed interest (see #26847).

sbrannen
reopened this on Apr 22, 2021
sbrannen
changed the title [-]Extend default script detection in @Sql to account for execution phase [SPR-13322][/-] [+]Extend default script detection in `@Sql` to account for execution phase [SPR-13322][/+] on Aug 19, 2021
djechelon

djechelon commented on Oct 1, 2021

@djechelon
djechelon
on Oct 1, 2021 · edited by djechelon
Contributor

Here are some notes

As the component to amend is SqlScriptsTestExecutionListener
 I suggest to make some order (both in the sense of clean up and sorting)

Proposal (theoretical)

Explicit and default scripts should be executed in a predefined symmetric order

TestClassName.sql if exists
Any other @Sql-explicit script (e.g. @Sql("my-script-name.sql") on class level with default phase BEFORE
TestClassName.method.sql if exists
TestClassName.method.before.sql (note 1)
Any other @Sql-explicit script (e.g. @Sql("my-script-name.sql") on method level with default phase BEFORE
(test method execution)
Any other @Sql-explicit script (e.g. @Sql("my-script-name.sql") on method level with default phase AFTER
TestClassName.method.after.sql if exists
Any other @Sql-explicit script (e.g. @Sql("my-script-name.sql") on class level with default phase AFTER
TestClassName.after.sql

Note 1: it doesn't make sense to use both TestClass.methodName.sql and TestClass.methodName.before.sql, but both files might exist in the predefined path, so really the implementor (🤚) must take this situation into account. One for example may choose to run only the first match found.

Proposed approach

Yea, the above sounds great, but we can achieve what is written in this proposal by reducing the impact and possible regressions.

So, I think that the executionPhase argument should be propagated to detectDefaultScript. In the before phase, both TestClass.methodName.sql and TestClass.methodName.before.sql should be tested, while the after phase will test for .after.

Without changing anything else, thus without guarantees to the symmetric order proposed above in case there are multiple Sql annotations, this should work.

Any feedback from @sbrannen and/or the OP?

djechelon
mentioned this on Oct 1, 2021
sbrannen

sbrannen commented on Oct 1, 2021

@sbrannen
sbrannen
on Oct 1, 2021
Member

Hi @djechelon,

Thanks for your input.

We will take that into account if we decide to implement this feature.

djechelon

djechelon commented on Oct 13, 2021

@djechelon
djechelon
on Oct 13, 2021
Contributor

Here is an implementation

I didn't run test yet, but it looks like the after test script detection works only when the @SQL annotation is present with phase set.

However, this implementation may have the least number of side effects and regressions over existing applications.

Convention over configuration is worth consideration (theoretical proposal above). Yet, I'm very interested in such a feature as I maintain a number of SQL scripts in my test classes.

Thanks for your time and consideration

sbrannen
added this to the General Backlog milestone on Oct 13, 2021
DarrMirr

DarrMirr commented on Sep 10, 2022

@DarrMirr
DarrMirr
on Sep 10, 2022

Hello @djechelon. Maybe Chained sql queries feature of DbChange JUnit extension helps you to manage with this issue?

You can use @Sql Spring Framework feature for tests where there are no requirements for default scripts and use chained sql queries of DbChange for tests where default scripts are needed. So, you can get benefits from both approaches.

As alternative way, you can also enclose tests with default scripts by classes (inner or top level):

@ExtendWith(DbChangeExtension.class)
@DbChangeOnce(sqlQueryFiles = "sql/database_init.sql")
@DbChangeOnce(sqlQueryFiles = "sql/database_destroy.sql", executionPhase = ExecutionPhase.AFTER_ALL)
@SqlExecutorGetter("defaultSqlExecutor")
public class DbChangeUsageTest {
    private DataSource dataSource;
    
    public DbChangeUsageTest() {
        dataSource = // code to create instance of DataSource 
    }
  
    public SqlExecutor defaultSqlExecutor() {
        return new DefaultSqlExecutor(dataSource);
    }
  
    @Test
    @DbChange(changeSet = InsertEmployee6Chained.class )
    @DbChange(changeSet = DeleteEmployee6Chained.class , executionPhase = DbChange.ExecutionPhase.AFTER_TEST)
    void changeSetChained() {
        /* code omitted */
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    in: testIssues in the test moduletype: enhancementA general enhancement

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @sbrannen@rstoyanchev@djechelon@spring-projects-issues@DarrMirr

        Issue actions
          Extend default script detection in `@Sql` to account for execution phase [SPR-13322] · Issue #17907 · spring-projects/spring-framework