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

MODELIX-721 Migrate cucumber tests for model-server to ktor tests #873

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

mhuster23
Copy link
Contributor

On my local machine the migrated test cases now only take 2s instead of 38s.
The saved 36s are roughly a 40% reduction of the previous total time to test the model-server.

To be verified by reviewers

  • Relevant public API members have been documented
  • Documentation related to this PR is complete
    • Boundary conditions are documented
    • Exceptions are documented
    • Nullability is documented if used
  • Touched existing code has been extended with documentation if missing
  • Code is readable
  • New features and fixed bugs are covered by tests

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Test Results

143 files  +2  143 suites  +2   10m 59s ⏱️ +14s
851 tests +1  842 ✅ +1  9 💤 ±0  0 ❌ ±0 
861 runs  +1  852 ✅ +1  9 💤 ±0  0 ❌ ±0 

Results for commit c473039. ± Comparison against base commit 601ef68.

This pull request removes 16 and adds 17 tests. Note that renamed tests count towards both.
Basic routes ‑ Default email after token is generated
Basic routes ‑ Heartbeat works
Basic routes ‑ Homepage works
Storing routes ‑ Events are received after subscription
Storing routes ‑ Events are received only for the key subscribed
Storing routes ‑ Get recursively
Storing routes ‑ Putting multiple keys
Storing routes ‑ Putting multiple keys, with some nulls, are recognized correctly
Storing routes ‑ Putting multiple keys, with some nulls, are stored correctly
Storing routes ‑ Retrieving forbidden key
…
org.modelix.model.server.V1ApiTest ‑ counter returns different ids for same key()
org.modelix.model.server.V1ApiTest ‑ default email after token is generated()
org.modelix.model.server.V1ApiTest ‑ events are only received for the subscribed key()
org.modelix.model.server.V1ApiTest ‑ events are received after subscription()
org.modelix.model.server.V1ApiTest ‑ keys can be retrieved recursively()
org.modelix.model.server.V1ApiTest ‑ multiple existing keys can be retrieved()
org.modelix.model.server.V1ApiTest ‑ multiple keys with non-null values are stored correctly()
org.modelix.model.server.V1ApiTest ‑ multiple keys with some null values are recognized correctly()
org.modelix.model.server.V1ApiTest ‑ multiple keys with some null values are stored correctly()
org.modelix.model.server.V1ApiTest ‑ multiple nonexistent keys can be retrieved()
…

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jul 2, 2024

JVM coverage report

Overall Project 52.01%
Files changed 96.27% 🍏

File Coverage
IndexPage.kt 100% 🍏
HealthApiImpl.kt 73.42% 🍏
Main.kt 20.5% -0.57%


class IndexPage {

fun init(application: Application) {

Check warning

Code scanning / detekt

The function init is missing documentation. Warning

The function init is missing documentation.
@mhuster23 mhuster23 marked this pull request as ready for review July 2, 2024 16:01
@mhuster23 mhuster23 requested review from slisson and a team as code owners July 2, 2024 16:01
Copy link
Contributor

@languitar languitar left a comment

Choose a reason for hiding this comment

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

Do we know the reasons for why some of the cucumber scenarios were commented out? Is it safe to drop them?

Apart from that, only minor things that I added.

After #862, kotest assertions would now also be available as an option.

@mhuster23 mhuster23 force-pushed the test/migrate-cucumber-to-ktor branch from 308c398 to e13c51b Compare July 3, 2024 13:24
@mhuster23 mhuster23 requested a review from languitar July 3, 2024 13:24
@mhuster23 mhuster23 force-pushed the test/migrate-cucumber-to-ktor branch from e13c51b to 4c3892c Compare July 4, 2024 08:30
@mhuster23 mhuster23 requested a review from odzhychko July 4, 2024 09:32
languitar
languitar previously approved these changes Jul 4, 2024
Copy link
Contributor

@languitar languitar left a comment

Choose a reason for hiding this comment

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

Ah, good idea with the plugin.

The only thing potentially left is that we have the exact same logic now copied to two tests, which could be deduplicated.

@mhuster23 mhuster23 merged commit 3725bfc into main Jul 4, 2024
19 checks passed
@mhuster23 mhuster23 deleted the test/migrate-cucumber-to-ktor branch July 4, 2024 12:55
@slisson
Copy link
Member

slisson commented Jul 5, 2024

🎉 This PR is included in version 8.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants