chore: fix run tests script

[alon-dotan-starkware]

Pull Request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build-related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this introduce a breaking change?

• Yes
• No

Other information

---

This change is 

[dorimedini-starkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @alon-dotan-starkware and @dan-starkware)

---

.github/workflows/main.yml line 81 at r1 (raw file):

  run-tests:
    if: github.event_name == 'pull_request'

why?

Code quote:

if: github.event_name == 'pull_request'

---

.github/workflows/main.yml line 86 at r1 (raw file):

      - uses: actions/checkout@v4
        with:
          fetch-depth: ${{ github.event_name == 'pull_request' && 2 || 0 }}

what's this?

Code quote:

        with:
          fetch-depth: ${{ github.event_name == 'pull_request' && 2 || 0 }}

---

.github/workflows/main.yml line 108 at r1 (raw file):

          ci/bin/pip install -r scripts/requirements.txt
          ci/bin/python scripts/run_tests.py --changes_only --commit_id HEAD^1
          ci/bin/python scripts/run_tests.py --changes_only --features concurrency --commit_id HEAD^1

what is the HEAD^?

Code quote:

          ci/bin/python scripts/run_tests.py --changes_only --commit_id HEAD^1
          ci/bin/python scripts/run_tests.py --changes_only --features concurrency --commit_id HEAD^1

---

scripts/run_tests.py line 36 at r1 (raw file):

        raise

    return [c.a_path for c in repo.head.commit.diff(commit_id)]

when I add another commit to a PR I expect tests to run relative to all files changed between the merge target and the tip of the branch, this is what's happening here?

Code quote:

def get_local_changes(repo_path, commit_id: Optional[str]) -> List[str]:
    os.environ["GIT_PYTHON_REFRESH"] = "quiet"  # noqa
    repo = Repo(repo_path)
    try:
        repo.head.object  # Check if local_repo is a git repo.
    except ValueError:
        print(f"unable to validate {repo_path} as a git repo.")
        raise

    return [c.a_path for c in repo.head.commit.diff(commit_id)]

[dorimedini-starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @alon-dotan-starkware and @dan-starkware)

---

.github/workflows/main.yml line 103 at r1 (raw file):

        run: echo "LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" >> $GITHUB_ENV
      - name: "Run tests pull request"
        if: github.event_name == 'pull_request'

isn't this always true if you're in run-tests?

Code quote:

if: github.event_name == 'pull_request'

[dorimedini-starkware]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @alon-dotan-starkware and @dan-starkware)

---

.github/workflows/main.yml line 103 at r1 (raw file):

Previously, dorimedini-starkware wrote…

isn't this always true if you're in run-tests?

still relevant... this if is also at the tpo of the entire step, when can we reach this point with github.event_name != 'pull_request'?

---

.github/workflows/main.yml line 93 at r2 (raw file):

          # in a pull request, we need 2 commits, the commit the pr born from on the target branch, and the merge one,
          # in pull we need the entire history.
          fetch-depth: ${{ github.event_name == 'pull_request' && 2 || 0 }}

fetch depth 0 means "all history"?

---

.github/workflows/main.yml line 123 at r2 (raw file):

        # See comments above.
        if: github.event_name == 'push'
        # TODO: Better support for running tests on push.

can you elaborate? what is the current issue?
also, since this is in the run-tests step which only runs when the event is a pull request, when will Run tests on push ever run?

[dorimedini-starkware]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @alon-dotan-starkware and @dan-starkware)

a discussion (no related file):
@alon-dotan-starkware this should be closed (we did something else)

---