Skip to content

Fixes #4520 #4954

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
Closed
wants to merge 1 commit into from
Closed

Fixes #4520 #4954

wants to merge 1 commit into from

Conversation

kenfinnigan
Copy link
Member

No description provided.

@kenfinnigan kenfinnigan added this to the 0.27.0 milestone Oct 29, 2019
jmartisk
jmartisk reviewed Oct 29, 2019
View reviewed changes
tcks/microprofile-metrics/rest/pom.xml
<MP_METRICS_TAGS>tier=integration</MP_METRICS_TAGS>
</environmentVariables>
<excludes>
<exclude>org.eclipse.microprofile.metrics.test.MpMetricTest</exclude>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is almost the whole TCK module. Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was only one test failing, due to the Gauge issue, but there's no way to exclude a single test in pom.xml

Copy link
Contributor

@jmartisk jmartisk Oct 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'm confused how you managed to get that test to fail, I thought this was an issue only for other TCKs that were compiled against Metrics 2.0. Btw I will send a PR to upgrade to Metrics 2.2 as soon as SmallRye Metrics 2.3 gets synced into Central

@kenfinnigan
Copy link
Member Author

kenfinnigan commented Oct 29, 2019 via email

jmartisk
jmartisk reviewed Oct 29, 2019
View reviewed changes
...st-client/deployment/src/main/java/io/quarkus/restclient/deployment/RestClientProcessor.java Outdated
private void processInterfaceReturnTypes(ClassInfo classInfo, Set<Type> returnTypes) {
for (MethodInfo method : classInfo.methods()) {
Type type = method.returnType();
if (!type.name().toString().contains("java.lang")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: startsWith would be safer than contains, you never know what kind of insane person could use a package that contains java.lang somewhere in the middle

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@jmartisk
Copy link
Contributor

Tbh I don’t know. If you can figure it out that’s awesome

It must have been some stale dependencies on your machine, I don't see any reason why this PR would break that test

@kenfinnigan
Copy link
Member Author

@jmartisk just verified I get the following error:

[ERROR] testListsAllJson(org.eclipse.microprofile.metrics.test.MpMetricTest)  Time elapsed: 0.305 s  <<< ERROR!
com.fasterxml.jackson.core.JsonParseException: 
Illegal unquoted character ((CTRL-CHAR, code 9)): has to be escaped using backslash to be included in string value
 at [Source: (String)"{"details":"Error handling fc40c116-670a-4bef-a309-0614e667bd59-1, java.lang.NumberFormatException: ","stack":"java.lang.NumberFormatException\n	at java.math.BigDecimal.&lt;init&gt;(BigDecimal.java:497)\n	at java.math.BigDecimal.&lt;init&gt;(BigDecimal.java:383)\n	at java.math.BigDecimal.&lt;init&gt;(BigDecimal.java:809)\n	at java.math.BigDecimal.valueOf(BigDecimal.java:1277)\n	at org.glassfish.json.JsonNumberImpl.getJsonNumber(JsonNumberImpl.java:46)\n	at org.glassfish.json.JsonProviderImpl.cre"[truncated 2535 chars]; line: 1, column: 146]
	at com.fasterxml.jackson.core.JsonParser._constructError(JsonParser.java:1804)

@geoand geoand mentioned this pull request Oct 29, 2019
Closed
@kenfinnigan kenfinnigan mentioned this pull request Oct 29, 2019
Closed
@gsmet
Copy link
Member

gsmet commented Oct 29, 2019

Closing, will be part of another PR.

@gsmet gsmet closed this Oct 29, 2019
@jmartisk jmartisk mentioned this pull request Oct 30, 2019
Closed
@jmartisk
Copy link
Contributor

@kenfinnigan ok, I reported #5014 for that and will look what can be done to re-enable it

@kenfinnigan kenfinnigan deleted the tck-rest branch October 30, 2019 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmartisk jmartisk jmartisk left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.27.0

Development

Successfully merging this pull request may close these issues.

3 participants

@kenfinnigan @jmartisk @gsmet