Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CDITCK-549 add test for extension registering custom OM which does no… #128

Closed
wants to merge 1 commit into from

Conversation

manovotn
Copy link
Contributor

…t override notify method.

@SpecAssertions({ @SpecAssertion(section = AFTER_BEAN_DISCOVERY, id = "ec") })
public void observerMethodNotOverridingNotifyMethodTreatedAsDefinitionError() {
deployer.deploy(CONFIGURATOR);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tremes not sure about this - do I need to call undeploy here? Since The deploy itself should not succeed I would guess not...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see compilation problem for ExtensionAddingCustomObserver but to your question - well maybe it would be safer to wrap this to try/catch block and do undeploy in the case of test failure (deployment will pass). This would also mean you can remove @ShouldThrowException.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh true, forgot to add refactoring onto the commit, sorry.
Will fix this and change the test accordingly.

@tremes
Copy link
Contributor

tremes commented Dec 14, 2016

Merged. Thank you!

@tremes tremes closed this Dec 14, 2016
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.

None yet

2 participants