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.

Sign up for GitHub

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

Jump to bottom

JBIDE-24963-6 - add new testclasses for CDI 2.0 #1958

Merged
merged 1 commit into from Aug 7, 2019
Merged

JBIDE-24963-6 - add new testclasses for CDI 2.0 #1958

merged 1 commit into from Aug 7, 2019

Conversation

zcervink
Copy link
Contributor

JBIDE-24963-6 - Add the following testclasses:

  • BeanDiscoveryInExplicitArchivesTest20.class
  • BeanDiscoveryInImplicitArchivesTest20.class
  • BeanParametersAnnotationTest20.class

Signed-off-by: Zbynek Cervinka zcervink@redhat.com

Jenkins: https://dev-platform-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/cdi20.itests/76/
Jira: https://issues.jboss.org/browse/JBIDE-24963

please review @jkopriva

@zcervink zcervink changed the title JBIDE-24963-6 - Add the following testclasses: JBIDE-24963-6 - Add the following testclasses Jul 18, 2019
@zcervink zcervink changed the title JBIDE-24963-6 - Add the following testclasses JBIDE-24963-6 - add new testclasses for CDI 2.0 Jul 20, 2019
@zcervink zcervink force-pushed the JBIDE-24963-6 branch 9 times, most recently from 3c6329a to 268f4d4 Compare July 24, 2019 09:31
@zcervink
Copy link
Contributor Author

testPR

2 similar comments
@zcervink
Copy link
Contributor Author

testPR

@zcervink
Copy link
Contributor Author

testPR

@zcervink zcervink force-pushed the JBIDE-24963-6 branch 3 times, most recently from b899668 to 41a14af Compare July 26, 2019 08:18
odockal
odockal requested changes Jul 29, 2019
View reviewed changes
tests/org.jboss.tools.cdi.bot.test/src/org/jboss/tools/cdi/bot/test/CDI20SuiteTest.java Outdated
@@ -41,6 +44,9 @@
AllAssignableDialogTestCDI20.class,
AssignableDialogFilterTestCDI20.class,
AsYouTypeValidationTestCDI20.class,
BeanDiscoveryInExplicitArchivesTest20.class,
Copy link
Member

Choose a reason for hiding this comment

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

Stick with naming convention and use CDI20.

Copy link
Member

Choose a reason for hiding this comment

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

Counts for every class of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

...g/jboss/tools/cdi/bot/test/beansxml/discovery/cdi11/BeanDiscoveryInImplicitArchivesTest.java
Marker validation = markers.get(0);
assertEquals(validation.getLineNumber(), 7);
assertTrue(validation.getText().contains("No bean is eligible for injection"));
super.validationOfBeanDiscoveryInImplicitArchives();
Copy link
Member

Choose a reason for hiding this comment

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

Add CDI11 pattern to the class name. Also, In new CDI20 test classes you have specified CDIVersion, add it to the CDI 11 classes as well, of course, value will be for 1.2 or 1.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CDIVersion specification add to the CDI 11 classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CDI11 pattern added to the class name.

...c/org/jboss/tools/cdi/bot/test/beansxml/discovery/cdi11/BeanDiscoveryInArchivesTemplate.java Outdated
assertEquals(1, markers.size());
Marker validation = markers.get(0);
assertEquals(validation.getLineNumber(), 8);
assertTrue(validation.getText().contains("No bean is eligible for injection"));
Copy link
Member

Choose a reason for hiding this comment

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

We will need to improve the message/problem check. Here was checked only problem's short message, now we have to distinguish between CDI11 and CDI20 - JSR-346 and JSR-365 specs. Some validator would solve the issue well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

...c/org/jboss/tools/cdi/bot/test/beansxml/discovery/cdi11/BeanDiscoveryInArchivesTemplate.java Outdated
assertEquals(1, markers.size());
Marker validation = markers.get(0);
assertEquals(validation.getLineNumber(), 7);
assertTrue(validation.getText().contains("No bean is eligible for injection"));
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@zcervink zcervink force-pushed the JBIDE-24963-6 branch 4 times, most recently from 5507ea2 to 6c67bcd Compare July 31, 2019 11:51
odockal
odockal requested changes Aug 2, 2019
View reviewed changes
...org/jboss/tools/cdi/bot/test/beansxml/annotation/cdi11/BeanParametersAnnotationTemplate.java Outdated
assertEquals(1, problems.size());
Problem validation = problems.get(0);
assertTrue(validation.getLocation().contains("7"));
assertTrue(validation.getDescription().contains("No bean is eligible for injection"));
Copy link
Member

Choose a reason for hiding this comment

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

Here also should be particular JSR number in warning messaged checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

odockal
odockal requested changes Aug 2, 2019
View reviewed changes
...org/jboss/tools/cdi/bot/test/beansxml/annotation/cdi11/BeanParametersAnnotationTemplate.java Outdated
* @author zcervink
*
*/
public class BeanParametersAnnotationTemplate extends CDITestBase {
Copy link
Member

Choose a reason for hiding this comment

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

move the class from foo.fooo.foooo.annotation.cdi11 to foo.fooo.foooo.annotation.template package. Same for any other template class that is misplaced in *.cdiXX folder. It should be place in its own template package within * (discovery, annotation) package. Check the other classes ;). This also count for any other PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved.

odockal
odockal requested changes Aug 6, 2019
View reviewed changes
...ss/tools/cdi/bot/test/beansxml/discovery/cdi20/BeanDiscoveryInExplicitArchivesTestCDI20.java Outdated
import org.jboss.ide.eclipse.as.reddeer.server.family.ServerMatcher;
import org.jboss.ide.eclipse.as.reddeer.server.requirement.ServerRequirement.JBossServer;
import org.jboss.tools.cdi.bot.test.CDITestBase;
import org.jboss.tools.cdi.bot.test.beansxml.discovery.cdi11.BeanDiscoveryInArchivesTemplate;
Copy link
Member

Choose a reason for hiding this comment

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

move template class into its own package as it is being used by both cdi version tests (1.1 and 2.0).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

...src/org/jboss/tools/cdi/bot/test/beansxml/annotation/cdi11/BeanParametersAnnotationTest.java Outdated
@@ -49,7 +31,7 @@
@JRE(cleanup=true)
@OpenPerspective(JavaEEPerspective.class)
@JBossServer(state=ServerRequirementState.PRESENT, cleanup=false)
public class BeanParametersAnnotationTest extends CDITestBase {
public class BeanParametersAnnotationTest extends BeanParametersAnnotationTemplate {
Copy link
Member

Choose a reason for hiding this comment

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

Add suffix CDI11 to this test class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@zcervink
Copy link
Contributor Author

zcervink commented Aug 6, 2019

Jenkins: https://dev-platform-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/cdi20.itests/121/

@odockal
Copy link
Member

odockal commented Aug 6, 2019

+1. Good work. @jkopriva Merge please. And consider giving me rights to this repo... :)

@zcervink zcervink force-pushed the JBIDE-24963-6 branch 2 times, most recently from 0820b71 to 79c1753 Compare August 6, 2019 15:50
@zcervink
JBIDE-24963-6 - Add the following testclasses:
aa9f224 
 - BeanDiscoveryInExplicitArchivesTest20.class
 - BeanDiscoveryInImplicitArchivesTest20.class
 - BeanParametersAnnotationTest20.class

Signed-off-by: Zbynek Cervinka <zcervink@redhat.com>
@jkopriva jkopriva merged commit 340dd87 into jbosstools:master Aug 7, 2019
@zcervink zcervink deleted the JBIDE-24963-6 branch August 7, 2019 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@odockal odockal odockal requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zcervink @odockal @jkopriva