bamaer commented on PR #9138:
URL: https://github.com/apache/hop/pull/9138#issuecomment-6490795580

   Thanks @vbhanuchander-lang, naming the listener class keeps the metas free 
of hop-ui and SWT without changing how the dialog behaves.
   
   What we ran on this branch:
   - All tests of the Oracle, MS SQL Server Native and Redshift plugins, 
including the `uitest` ones, plus the `*DisabledWidgetsTest` of 9 other 
database plugins and `DisabledGuiWidgetsNpeTest`: all pass.
   - Without hop-ui and SWT on the classpath: `HopEnvironment.init()`, then 
save, load and build the URL of an Oracle, MSSQL native and Redshift 
connection. That works here. With the 2.20.0 jars it fails in 
`HopEnvironment.init()` with `NoClassDefFoundError: 
IGuiPluginCompositeWidgetsListener`.
   - A throwaway SWTBot test that opens the real `DatabaseMetaEditor` for the 
three types: fields hide and disable as before, also after switching the 
connection type away and back.
   
   No blockers.
   
   **Should fix**
   - The new path in `DatabaseMetaEditor.pluginWidgetsListener()` / 
`createPluginWidgetsListener()` 
(`ui/src/main/java/org/apache/hop/ui/core/database/DatabaseMetaEditor.java:525`)
 has no test: `OracleWidgetVisibilityTest` and `DisabledGuiWidgetsTestBase` 
create the listener themselves, and no `uitest` opens the editor. Suggest a 
`@Tag("uitest")` test per plugin that opens `DatabaseMetaEditor`, changes a 
field, checks what is hidden or disabled, and switches the connection type away 
and back. Redshift has no UI test at all; a `RedshiftDisabledWidgetsTest` like 
`OracleDisabledWidgetsTest` is three lines and passes (18 cases).
   
   **Out of scope for this PR**
   - The same pattern exists outside the database plugins: 
`AwsSecretsManagerVariableResolver`, the Vault/OpenBao resolvers and 
`SmbConnection` implement `IGuiPluginCompositeWidgetsListener` themselves, so 
they likely break headless use the same way when those plugins are on the 
classpath (not run). `getGuiWidgetsListenerClassName()` only covers 
`IDatabase`, so that needs its own issue and PR; it shouldn't hold this one up.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to