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

Feature/create artemis test module #5900

Merged

Conversation

uaihebert
Copy link
Contributor

Fixes #3833
The idea of this pr is:

  1. create the test module with ArtemisResourceTest
  2. expose it to be used by the developers that use Quarkus
  3. replace integration tests that used the test class

Copy link
Member

@aguibert aguibert left a comment

Choose a reason for hiding this comment

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

Overall looks good, my only comment is on placement of the new module

</parent>

<modelVersion>4.0.0</modelVersion>
<artifactId>quarkus-artemis-test</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

we should put this test module under quarkus/test-framework/ with the other test modules, and set the pom parent to quarkus-test-framework

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

I agree with @aguibert's comment in https://github.com/quarkusio/quarkus/pull/5900/files#r352410389. The module should be moved under quarkus-test-framework

@gsmet
Copy link
Member

gsmet commented Dec 2, 2019

Agreed and you will need to name it quarkus-test-artemis to be consistent with the others.

@uaihebert
Copy link
Contributor Author

Thank you for the feedback @aguibert @gastaldi @gsmet

I updated it as requested.

@gastaldi
Copy link
Contributor

gastaldi commented Dec 3, 2019

That looks good. Can you squash all your commits in a single one? Thank you!

@uaihebert uaihebert force-pushed the feature/create-artemis-test-module branch from 3358056 to 51179a7 Compare December 3, 2019 22:27
@uaihebert
Copy link
Contributor Author

@gastaldi sure, no problem.

@uaihebert uaihebert force-pushed the feature/create-artemis-test-module branch from d4e99a5 to 5a8448a Compare December 3, 2019 22:49
@uaihebert uaihebert force-pushed the feature/create-artemis-test-module branch from 5a8448a to 93bba85 Compare December 3, 2019 22:56
@gastaldi gastaldi added this to the 1.1.0 milestone Dec 4, 2019
@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 4, 2019
@uaihebert
Copy link
Contributor Author

hello @gastaldi the build failed on the windows test.

any tip for what should I look in the log? the logs are huge and I could not find the error in there.

@gastaldi
Copy link
Contributor

gastaldi commented Dec 4, 2019

@uaihebert it doesn't seem related to your changes, let me see if I can trigger the build again

@uaihebert
Copy link
Contributor Author

hello @gastaldi indeed it was not related.

let me know if you need anything else.

@gastaldi gastaldi merged commit 47ed92b into quarkusio:master Dec 4, 2019
@gastaldi
Copy link
Contributor

gastaldi commented Dec 4, 2019

@uaihebert Merged. Thank you!

@uaihebert uaihebert deleted the feature/create-artemis-test-module branch December 4, 2019 13:24
@uaihebert
Copy link
Contributor Author

thank you too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArtemisTestResource available
4 participants