search

quarkus-qe / quarkus-test-framework

Write your test once and run it everywhere!
Apache License 2.0
14 stars 26 forks source link

Add CLI feature to update quarkus app #1188

Closed mocenas closed 1 week ago

mocenas commented 1 week ago

Summary

We intend to test update of quarkus using CLI tool. Expose the update feature in the framework.

Please check the relevant options

Checklist:

mocenas commented 1 week ago

it's ok for now. Please keep in mind that you will need WAY more and when you will open these follow-up PRs in FW for the update command, we will need tests. At least basic ones. Thank you

RN I'm not really sure what will be required in FW, so I created just a basic update method, so we can start writing actual tests in the TS. Quarkus update command doesn't have many more CLI options for update and I'm not sure we will need them at all (but never say never).

Should we actually write tests for this in FW? As I see, quarkus-test-cli module has no tests at all, and those tests would probably duplicate what we (will) have in TS.

gtroitsk commented 1 week ago

it's ok for now. Please keep in mind that you will need WAY more and when you will open these follow-up PRs in FW for the update command, we will need tests. At least basic ones. Thank you

RN I'm not really sure what will be required in FW, so I created just a basic update method, so we can start writing actual tests in the TS. Quarkus update command doesn't have many more CLI options for update and I'm not sure we will need them at all (but never say never).

Should we actually write tests for this in FW? As I see, quarkus-test-cli module has no tests at all, and those tests would probably duplicate what we (will) have in TS.

I think it will be good to have a simple example scenario for quarkus app update in FW. Some tests for CLI are in QuarkusCliClientIT.java.

michalvavrik commented 1 week ago

Should we actually write tests for this in FW?

If you make changes to FW that you cannot verify with FW CI, we cannot know whether changes to FW are correct without running TS which is not automated with FW changes. IMO the FW tests should verify functionality of FW while TS tests should verify Quarkus.

That said, sometimes it is not feasible to test every PR and not all my PRs have tests. I've already approved.

As I see, quarkus-test-cli module has no tests at all

Here are tests https://github.com/quarkus-qe/quarkus-test-framework/tree/main/examples/quarkus-cli.

and those tests would probably duplicate what we (will) have in TS.

In TS you will want to verify relevant Quarkus release migrations, here you just want to verify that what your code says it does. It is different because you don't care about specific changes but about specific functionality. I cannot tell you whether there will be duplication because I don't know your test coverage :-) So I don't mind little duplication.

michalvavrik commented 1 week ago

we can start writing actual tests in the TS

What I meant was that this PR is not sufficient to submit tests in the TS and you don't need FW release to start writing tests in the TS with snapshot FW. This PR is completely fine, thank you.

mocenas commented 1 week ago

I've added a test for update feature. Sorry it didn't dawn on me, that "examples" dir is containing actual tests for the framework.

michalvavrik commented 1 week ago

I've added a test for update feature. Sorry it didn't dawn on me, that "examples" dir is containing actual tests for the framework.

I dropped tests from Quarkus CLI module just a month ago https://github.com/quarkus-qe/quarkus-test-framework/pull/1164 :-D So you were not that wrong.