Skip to content

Commit

Permalink
[forge] Print dashboard link after running
Browse files Browse the repository at this point in the history
Print the dashboard link for the run after finishing even when running locally

Also move the debugging output above the summary since its very confusing to
see debugging output instead of result.

Also fix some string interpolation bugs :P

Test Plan: run locally, see the dashboard link :D
  • Loading branch information
perryjrandall committed Sep 7, 2022
1 parent 0db2b59 commit 86bc471
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 63 deletions.
45 changes: 45 additions & 0 deletions .github/workflows/forge-unittests.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
name: "Run Forge Wrapper Unittests"

on:
workflow_call:
inputs:
GIT_SHA:
required: true
type: string

jobs:
forge-wrapper-unittests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
ref: ${{ inputs.GIT_SHA }}
# Get enough commits to compare to
fetch-depth: 100

- name: Get changed files
id: changed-files
uses: tj-actions/[email protected]

- name: Should run tests
run: |
set -x
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
[[ $file =~ .*testsuite/.*.py ]] && echo "::set-output name=SHOULD_RUN::true" && echo "Running unittests"
done
exit 0
id: should-run-tests

- uses: actions/setup-python@v4
if: steps.should-run-tests.outputs.SHOULD_RUN == 'true'

- name: Install python deps
if: steps.should-run-tests.outputs.SHOULD_RUN == 'true'
run: pip3 install click==8.1.3 psutil==5.9.1

- name: Run forge wrapper tests
if: steps.should-run-tests.outputs.SHOULD_RUN == 'true'
run: python -m unittest testsuite/forge_test.py
37 changes: 3 additions & 34 deletions .github/workflows/lint-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -199,37 +199,6 @@ jobs:
retention-days: 1

forge-unittests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
ref: ${{ inputs.GIT_SHA }}
# Get enough commits to compare to
fetch-depth: 100

- name: Get changed files
id: changed-files
uses: tj-actions/[email protected]

- name: Should run tests
run: |
set -x
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
[[ $file =~ .*testsuite/.*.py ]] && echo "::set-output name=SHOULD_RUN::true" && echo "Running unittests"
done
exit 0
id: should-run-tests

- uses: actions/setup-python@v4
if: steps.should-run-tests.outputs.SHOULD_RUN == 'true'

- name: Install python deps
if: steps.should-run-tests.outputs.SHOULD_RUN == 'true'
run: pip3 install click==8.1.3 psutil==5.9.1

- name: Run forge wrapper tests
if: steps.should-run-tests.outputs.SHOULD_RUN == 'true'
run: python -m unittest testsuite/forge_test.py
uses: ./.github/workflows/forge-unittests.yaml
with:
GIT_SHA: ${{ inputs.GIT_SHA }}
3 changes: 2 additions & 1 deletion .github/workflows/run-forge.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ on:
type: string
description: The Forge k8s namespace to be used for test. This value should manage Forge test concurrency. It may be truncated.
FORGE_CLUSTER_NAME:
# HACK: Undo this after decommission old clusters
default: aptos-forge-big-0
required: false
type: string
default: aptos-forge-big-0
description: The Forge k8s cluster to be used for test
FORGE_RUNNER_DURATION_SECS:
required: false
Expand Down
6 changes: 3 additions & 3 deletions testsuite/fixtures/testMain.fixture

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 35 additions & 25 deletions testsuite/forge.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,15 @@ def set_output(self, output: str) -> None:
def set_debugging_output(self, output: str) -> None:
self.debugging_output = output

def format(self) -> str:
output = "\n".join([
def format(self, context: ForgeContext) -> str:
output_lines = []
if not self.succeeded():
output_lines.append(self.debugging_output)
output_lines.extend([
f"Forge output: {self.output}",
f"Forge {self.state.value.lower()}ed",
])
if not self.succeeded():
output += "\n" + self.debugging_output
return output
return "\n".join(output_lines)

def succeeded(self) -> bool:
return self.state == ForgeState.PASS
Expand Down Expand Up @@ -690,16 +691,19 @@ def get_humio_logs_link(forge_namespace: str) -> str:


def format_github_info(context: ForgeContext) -> str:
return (
textwrap.dedent(
f"""
* [Test runner output]({context.github_job_url})
* Test run is {'' if context.forge_blocking else 'not '}land-blocking
"""
if not context.github_job_url:
return ""
else:
return (
textwrap.dedent(
f"""
* [Test runner output]({context.github_job_url})
* Test run is {'' if context.forge_blocking else 'not '}land-blocking
"""
)
.lstrip()
.strip()
)
.lstrip()
.strip()
)


def format_pre_comment(context: ForgeContext) -> str:
Expand Down Expand Up @@ -988,12 +992,12 @@ def list_eks_clusters(shell: Shell) -> List[str]:
cluster_json = shell.run(["aws", "eks", "list-clusters"]).unwrap()
# This type annotation is not enforced, just helpful
try:
cluster_result: ListClusterResult = json.loads(cluster_json)
return [
cluster_name
for cluster_name in cluster_result["clusters"]
if cluster_name.startswith("aptos-forge-big-")
]
cluster_result: ListClusterResult = json.loads(cluster_json.decode())
clusters = []
for cluster_name in cluster_result["clusters"]:
if cluster_name.startswith("aptos-forge-"):
clusters.append(cluster_name)
return clusters
except Exception as e:
raise AwsError("Failed to list eks clusters") from e

Expand Down Expand Up @@ -1318,7 +1322,7 @@ def test(
try:
current_cluster = get_current_cluster_name(shell)
except Exception as e:
print("Warning: failed to get current cluster name: {e}")
print(f"Warning: failed to get current cluster name: {e}")

if not forge_cluster_name or balance_clusters:
cluster_names = list_eks_clusters(shell)
Expand Down Expand Up @@ -1417,7 +1421,7 @@ def test(
forge_cluster_name=forge_cluster_name,
forge_blocking=forge_blocking == "true",
github_actions=github_actions,
github_job_url=f"{github_server_url}/{github_repository}/actions/runs/{github_run_id}",
github_job_url=f"{github_server_url}/{github_repository}/actions/runs/{github_run_id}" if github_run_id else None,
forge_args=forge_args,
)
forge_runner_mapping = {
Expand All @@ -1432,6 +1436,8 @@ def test(
ForgeResult.empty(),
[ForgeFormatter(forge_pre_comment, lambda *_: pre_comment)],
)
else:
print(pre_comment)

if forge_runner_mode == "pre-forge":
return
Expand All @@ -1443,19 +1449,23 @@ def test(
forge_runner = forge_runner_mapping[forge_runner_mode]()
result = forge_runner.run(context)

print(result.format())

outputs = []
if forge_output:
outputs.append(ForgeFormatter(forge_output, lambda *_: result.output))
if forge_report:
outputs.append(ForgeFormatter(forge_report, format_report))
else:
print(format_report(context, result))
if forge_comment:
outputs.append(ForgeFormatter(forge_comment, format_comment))
else:
print(format_comment(context, result))
if github_step_summary:
outputs.append(ForgeFormatter(github_step_summary, format_comment))
context.report(result, outputs)

print(result.format(context))

if not result.succeeded() and forge_blocking == "true":
raise SystemExit(1)

Expand All @@ -1464,7 +1474,7 @@ def test(
try:
set_current_cluster(shell, current_cluster)
except Exception as ee:
print("Warning: failed to restore current cluster: {ee}")
print(f"Warning: failed to restore current cluster: {ee}")
print("Set cluster manually with aws eks update-kubeconfig --name {current_cluster}")
raise Exception(
"Forge state:\n" + dump_forge_state(shell, forge_namespace)
Expand Down

0 comments on commit 86bc471

Please sign in to comment.