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

Update to use ktor 2.0.1 #250

Merged
merged 3 commits into from
May 10, 2022
Merged

Update to use ktor 2.0.1 #250

merged 3 commits into from
May 10, 2022

Conversation

MikAoJk
Copy link
Contributor

@MikAoJk MikAoJk commented May 10, 2022

No description provided.

@MikAoJk MikAoJk marked this pull request as ready for review May 10, 2022 08:47
@MikAoJk MikAoJk requested a review from a team as a code owner May 10, 2022 08:47
@MikAoJk
Copy link
Contributor Author

MikAoJk commented May 10, 2022

plz run tests
Localy there are some tests that fails, not sure if local issue or not

Copy link
Contributor

@ybelMekk ybelMekk left a comment

Choose a reason for hiding this comment

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

Hello. Jawk

Tests runs fine locally.

But i get som deprecation notice:

mock-oauth2-server/src/test/kotlin/examples/kotlin/ktor/resourceserver/OAuth2ResourceServerAppTest.kt: (18, 13): 
withTestApplication(Application.() -> Unit, TestApplicationEngine.() -> R): 
R' is deprecated. Please use new `testApplication` API: https://ktor.io/docs/migrating-2.html#testing-api

also (35, 13) and (55, 13)

@MikAoJk
Copy link
Contributor Author

MikAoJk commented May 10, 2022

Yes, The withTestApplication/withApplication functions are replaced with a new testApplication function. Should we include the change in this PR or in a new one?

Ref:
https://ktor.io/docs/migrating-2.html#testing-api

@ybelMekk
Copy link
Contributor

Yes, The withTestApplication/withApplication functions are replaced with a new testApplication function. Should we include the change in this PR or in a new one?

Ref: https://ktor.io/docs/migrating-2.html#testing-api

I recon it could be included in this PR, as it is spinns of after the update to ktor 2.0.1.

@ybelMekk ybelMekk added the enhancement New feature or request label May 10, 2022
@MikAoJk MikAoJk requested a review from ybelMekk May 10, 2022 10:09
@MikAoJk
Copy link
Contributor Author

MikAoJk commented May 10, 2022

@ybelMekk i fixed the usage of withTestApplication due to deprecation, can you run the tests now, and see if its all good?

Copy link
Contributor

@ybelMekk ybelMekk left a comment

Choose a reason for hiding this comment

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

Tests run 🆗

@MikAoJk did you run formatKotlin ?

@MikAoJk
Copy link
Contributor Author

MikAoJk commented May 10, 2022

no i will do

@MikAoJk
Copy link
Contributor Author

MikAoJk commented May 10, 2022

My local issue was due to this content in the /etc/hosts file:
127.0.0.1 view-localhost
127.0.0.1 localhost

i need to remove the view-localhost and the the tests worked

@MikAoJk MikAoJk requested a review from ybelMekk May 10, 2022 10:37
@MikAoJk
Copy link
Contributor Author

MikAoJk commented May 10, 2022

@ybelMekk formatKotlin done

@ybelMekk ybelMekk requested a review from tommytroen May 10, 2022 11:00
@ybelMekk
Copy link
Contributor

@tommytroen 👀?

Copy link
Collaborator

@tommytroen tommytroen left a comment

Choose a reason for hiding this comment

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

lgtm. 🚢

@ybelMekk ybelMekk merged commit 4dd5c9d into navikt:master May 10, 2022
@MikAoJk MikAoJk deleted the ktor2 branch May 10, 2022 13:18
@ybelMekk ybelMekk added dependencies and removed enhancement New feature or request labels May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants