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

Show suggestion on unknown target #1477

Merged
merged 17 commits into from
Dec 15, 2020
Merged

Conversation

jawira
Copy link
Contributor

@jawira jawira commented Dec 14, 2020

This adds the message Did you mean 'target'? when target is not found.

For example:

<?xml version="1.0" encoding="UTF-8" ?>

<project name="demo" default="hello">
    <target name="setup">
        <echo>This is setup</echo>
    </target>
    <target name="hello">
        <echo>This is hello</echo>
    </target>
    <target name="clean">
        <echo>This is clean</echo>
    </target>
</project>
$ bin/phing hell
Buildfile: /home/jawira/PhpstormProjects/fork-phing/build.xml

BUILD FAILED
 Target 'hell' does not exist in this project. Did you mean 'hello'?

Total time: 0.1012 seconds

This was some kind of pet feature for me, so don't hesitate to close this PR if you think it's not useful :)

@codecov
Copy link

codecov bot commented Dec 15, 2020

Codecov Report

Merging #1477 (feeb97d) into master (2d04974) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1477      +/-   ##
============================================
+ Coverage     52.91%   52.92%   +0.01%     
- Complexity     9217     9222       +5     
============================================
  Files           470      470              
  Lines         22375    22385      +10     
============================================
+ Hits          11840    11848       +8     
- Misses        10535    10537       +2     
Impacted Files Coverage Δ Complexity Δ
classes/phing/Project.php 79.81% <100.00%> (+0.63%) 132.00 <4.00> (+5.00)
classes/phing/tasks/ext/UnzipTask.php 77.27% <0.00%> (-4.55%) 5.00% <0.00%> (ø%)
classes/phing/tasks/ext/HttpTask.php 88.88% <0.00%> (-2.23%) 19.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d04974...feeb97d. Read the comment docs.

@jawira jawira force-pushed the did-you-mean branch 3 times, most recently from 188073a to 481b95d Compare December 15, 2020 13:45
@jawira
Copy link
Contributor Author

jawira commented Dec 15, 2020

Hi, I had some issues with Github Actions. It seems that:

  • XDebug3 is used now, so I had to add XDEBUG_MODE environment variable.
  • ocular.phar is not compatible with PHP 8 (guzzle incompatiblity)

I did my best to fix those since I have no experience with Github Actions.


- name: Transfer coverage
if: github.repository_owner == 'phingofficial'
with:
php-version: '7.4'
run: |
Copy link
Member

@siad007 siad007 Dec 15, 2020

Choose a reason for hiding this comment

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

@jawira you cannot have both with and run in one step.
I would suggest to go by:

  coverage:
    runs-on: ubuntu-latest
    needs:
      - static_code_analysis
    steps:
      - name: Checkout repository
        uses: actions/checkout@v2

      - name: Setup PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '7.4'
          ini-values: xdebug.mode=coverage

      - name: Get composer cache directory
        id: composer-cache
        run: echo "::set-output name=dir::$(composer config cache-files-dir)"

      - name: Cache dependencies
        uses: actions/cache@v2
        with:
          path: ${{ steps.composer-cache.outputs.dir }}
          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
          restore-keys: ${{ runner.os }}-composer-

      - name: Composer install
        run: composer install --no-interaction --no-progress --ansi

      - name: Test with coverage
        working-directory: test
        run: |
          echo "=== SETTING GIT IDENTITY ==="
          git config --global user.email "github-ci-build@phing.info"
          git config --global user.name "Phing Github Action"
          echo "=== RUN TESTS ==="
          ../bin/phing -debug -Dtests.codecoverage=true -listener "phing.listener.StatisticsListener"

      - name: Transfer coverage
        if: github.repository_owner == 'phingofficial'
        run: |
          bash <(curl -s https://codecov.io/bash) -f ./test/reports/clover-coverage.xml
          wget https://scrutinizer-ci.com/ocular.phar && php ocular.phar code-coverage:upload --format=php-clover ./test/reports/clover-coverage.xml

@siad007
Copy link
Member

siad007 commented Dec 15, 2020

* `ocular.phar` is not compatible with PHP 8 (guzzle incompatiblity)

AFAIK we are using PHP 7.4 for code-coverage step?! Where exactly did you have a problem with PHP 8?

@jawira
Copy link
Contributor Author

jawira commented Dec 15, 2020

* `ocular.phar` is not compatible with PHP 8 (guzzle incompatiblity)

AFAIK we are using PHP 7.4 for code-coverage step?! Where exactly did you have a problem with PHP 8?

No, it wasn't PHP 7.4, by default this step used PHP 8 I added php -v to be sure.

@siad007
Copy link
Member

siad007 commented Dec 15, 2020

Ah ok - I see. There was a change in the OS ubuntu-latest was updated. So yes you are right. I will change to code-coverage with PHP 8.0 in a later PR.

@siad007
Copy link
Member

siad007 commented Dec 15, 2020

@jawira please check the new steps in my last suggestion - we have to add php 7.4 in a new single step and have the additional composer steps also (just take over my snipped)

@jawira jawira marked this pull request as draft December 15, 2020 22:01
siad007 added a commit that referenced this pull request Dec 15, 2020
@siad007
Copy link
Member

siad007 commented Dec 15, 2020

There was a wrong step in my post:

      - name: Composer install
        run: composer install --no-interaction --no-progress --ansi

      - name: Test with coverage
        working-directory: test
        run: |
          echo "=== SETTING GIT IDENTITY ==="
          git config --global user.email "github-ci-build@phing.info"
          git config --global user.name "Phing Github Action"
          echo "=== RUN TESTS ==="
          ../bin/phing -debug -Dtests.codecoverage=true -listener "phing.listener.StatisticsListener"

After that change I would merge.

@siad007 siad007 marked this pull request as ready for review December 15, 2020 22:38
@siad007
Copy link
Member

siad007 commented Dec 15, 2020

@jawira thanks for your PR I will merge it and have a look in the build pipeline if something strange should occure in master.

@siad007 siad007 merged commit 67acf77 into phingofficial:master Dec 15, 2020
@jawira jawira deleted the did-you-mean branch December 15, 2020 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants