search

acquia / cli

Command-line interface for Acquia Cloud Platform products
https://docs.acquia.com/acquia-cli/
GNU General Public License v2.0
42 stars 47 forks source link

GL-2039: Added prompt for nodejs and php project selection. #1683

Closed akashkska closed 6 months ago

akashkska commented 6 months ago

Codestudio is introducing feature to create node js project separately, hence implemented prompt to choose type of project and respective flow for project creation.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.29%. Comparing base (d7af9cf) to head (e878acf).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1683 +/- ## ============================================ + Coverage 91.15% 91.29% +0.14% - Complexity 1784 1794 +10 ============================================ Files 122 122 Lines 6366 6448 +82 ============================================ + Hits 5803 5887 +84 + Misses 563 561 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

anavarre commented 6 months ago

@akashkska action item on you. Will you be able to work on the PR in the current sprint?

akashkska commented 6 months ago

@akashkska action item on you. Will you be able to work on the PR in the current sprint?

yes working on same. There are some understanding gaps related to mutation testing so it is taking time to resolve .

danepowell commented 6 months ago

To get you started, I recorded myself killing one of the mutants. Try replicating this process to kill a few more. You don't need to kill 100% of them, but get as many as you can.

https://github.com/acquia/cli/assets/1984514/101df98f-6753-4e79-ad27-ae5dbd24f823

akashkska commented 6 months ago

To get you started, I recorded myself killing one of the mutants. Try replicating this process to kill a few more. You don't need to kill 100% of them, but get as many as you can.

Screen.Recording.2024-02-21.at.3.39.17.PM.mov

Is this correct recording ? I can see 3 sec recording.

danepowell commented 6 months ago

Sorry, indeed it was the wrong recording. I reuploaded it.

akashkska commented 6 months ago

@danepowell After latest commit No mutation warnings are shown in change log, also I see 2 mutants were killed. So does that mean PR is good to go in main ? If yes will you please approve and merge.

danepowell commented 6 months ago

Sorry, sometimes the tests time out. I reran them and will increase the timeout in master.

I think you'd only need to add 2-3 assertions to kill all or almost all of the mutants, like I did in the video. Please try it. It's not just busy work; the escaped mutants indicate the tests are not adequate, and I need to be able to rely on the tests in order to maintain this code.

akashkska commented 6 months ago

Apologise @danepowell fix might be silly, to add valid assertions. But for me its taking time due to lack of expertise on php, as this is first time I am interacting with php codebase and testcases.

anavarre commented 6 months ago

Dane is back at work today and will be able to further guide you on mutation testing. Good work thus far on addressing them.

danepowell commented 6 months ago

Thanks for all of the work on this; it needs some stylistic cleanup but I can follow up with that (we should have codesniffer rules for these things).