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.

Sign up for GitHub

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

Add a Github workflow to test pip/conda package installation #353

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Add a Github workflow to test pip/conda package installation #353

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45efb35
(feat) Add a Github workflow to test pip and conda installation
shibaeff Sep 28, 2024
d5160e3
(feat) Add workflow to test local pip installation
shibaeff Oct 2, 2024
2f93221
Revert "(feat) Add a Github workflow to test pip and conda installation"
shibaeff Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/test-pip.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Test package installation by running a basic example

on:
pull_request:

jobs:
test-pip:
runs-on: ubuntu-latest
strategy:
matrix:
platform: [linux/amd64]
Comment on lines +6 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider expanding the matrix strategy.

The job configuration is good, but the matrix strategy currently only includes one platform (linux/amd64). Consider expanding this to test on multiple operating systems and Python versions for more comprehensive coverage.

You could modify the matrix strategy as follows:

strategy:
  matrix:
    os: [ubuntu-latest, macos-latest, windows-latest]
    python-version: [3.8, 3.9, '3.10', '3.11']

Then, update the runs-on to use ${{ matrix.os }} and the Python setup step to use ${{ matrix.python-version }}.


steps:
- name: Checkout code
uses: actions/checkout@v3

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: 3.11

- name: Clone sdk-python repository
run: |
git clone https://github.com/InjectiveLabs/sdk-python
cd sdk-python

- name: Install injective-py from local copy
run: |
pip install .

- name: Run Python script
run: |
python3 examples/chain_client/tendermint/query/3_GetLatestBlock.py
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance the script execution step.

The current script execution step has a few potential improvements:

  1. The script path is hardcoded, which could break if the repository structure changes.
  2. There's no error handling or output capturing.

Consider the following enhancements:

- name: Run Python script
  run: |
    SCRIPT_PATH="examples/chain_client/tendermint/query/3_GetLatestBlock.py"
    if [ -f "$SCRIPT_PATH" ]; then
      python3 "$SCRIPT_PATH"
    else
      echo "Error: Script not found at $SCRIPT_PATH"
      exit 1
    fi
  continue-on-error: true

- name: Check script execution
  run: |
    if [ ${{ steps.run_script.outcome }} == 'failure' ]; then
      echo "Script execution failed. Please check the logs for details."
      exit 1
    fi

This modification:

  1. Uses a variable for the script path, making it easier to update.
  2. Checks if the script exists before attempting to run it.
  3. Uses continue-on-error to ensure the workflow doesn't stop immediately on script failure.
  4. Adds a separate step to check the outcome and provide appropriate feedback.

Loading