-
Notifications
You must be signed in to change notification settings - Fork 45
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
Automatically run tasks in CI if task is in a non-terminal state #337
Comments
Hey, I'm wondering if I could do that, but I'm not sure, if I'm understanding this task correclty. Am I right in thinking that the job is to modify ./circleci/config.yml file by adding a new job to the workflow? The job involves listing all non-terminal tasks, and running a simulation for each, as defined in the README.md file of the task? Is that correct? |
Hey! Yes that's close. So you'll see the Something like this diff is how I'd approach it. Let me know if you have any more questions, and if you want to go ahead implementing this! Would love to assign the issue to you if you're interested :) diff --git a/script/utils/check-task-statuses.sh b/script/utils/check-task-statuses.sh
index 89c77c5..4494383 100644
--- a/script/utils/check-task-statuses.sh
+++ b/script/utils/check-task-statuses.sh
@@ -4,6 +4,9 @@ set -euo pipefail
VALID_STATUSES=("DRAFT, NOT READY TO SIGN" "CONTINGENCY TASK, SIGN AS NEEDED" "READY TO SIGN" "SIGNED" "EXECUTED" "CANCELLED")
errors=() # We collect all errors then print them at the end.
+# Array holding paths of tasks ready to simulate.
+ready_to_simulate=()
+
# Function to check status and hyperlinks for a single file.
check_status_and_hyperlinks() {
local file_path=$1
@@ -40,6 +43,11 @@ check_status_and_hyperlinks() {
errors+=("Error: Status is EXECUTED but no link to transaction found in $file_path")
fi
fi
+
+ # If a file is not in a terminal state, add it to the ready_to_simulate array.
+ if [[ "$status_line" != *"SIGNED"* && "$status_line" != *"EXECUTED"* && "$status_line" != *"CANCELLED"* ]]; then
+ ready_to_simulate+=("$file_path")
+ fi
}
# Find README.md files for all tasks and process them.
@@ -57,3 +65,16 @@ if [[ ${#errors[@]} -gt 0 ]]; then
else
echo "✅ All task statuses are valid"
fi
+
+# Execute all tasks that are ready to simulate.
+if [[ ${#ready_to_simulate[@]} -gt 0 ]]; then
+ echo "Simulating tasks:"
+ for path in "${ready_to_simulate[@]}"; do
+ echo " Simulating $path"
+ # Check if the task is nested or single. We can usually do this by looking
+ # at the name of the Solidity file in this directory. If it's SignFromJson.s.sol
+ # it's single, and if it's NestedSignFromJson.s.sol it's nested.
+ # For nested tasks we should sign as both the foundation and the council.
+ # TODO implement the above.
+ done
+fi |
Thanks for the answer! ;) I've already listed all non-terminal tasks. I've made some changes, such as:
Let’s say I have all the non-terminal tasks, and I want to simulate them. Should there be a universal simulation pattern: one for nested tasks and another for single tasks? Currently, we have the following (these folders were provided by my listing function): In the folder: In the folder: In the folder: Am I correct in thinking that ./SINGLE.md and ./NESTED.md contain direct, universal guidelines for single and nested tasks, respectively? Should all subsequent tasks be simulated based on one of these two files? If so, what should we do with tasks in the following folders: './tasks/eth/fp-recovery/...' ,'./tasks/sep/fp-recovery/...' and './tasks/sep/001-op-extended-pause'? Current changes I've made: diff --git a/script/utils/check-task-statuses.sh b/script/utils/check-task-statuses.sh
index 89c77c5..8c09e67 100644
--- a/script/utils/check-task-statuses.sh
+++ b/script/utils/check-task-statuses.sh
@@ -1,9 +1,19 @@
#!/bin/bash
set -euo pipefail
-VALID_STATUSES=("DRAFT, NOT READY TO SIGN" "CONTINGENCY TASK, SIGN AS NEEDED" "READY TO SIGN" "SIGNED" "EXECUTED" "CANCELLED")
+NON_TERMINAL_STATUSES=("DRAFT, NOT READY TO SIGN" "CONTINGENCY TASK, SIGN AS NEEDED" "READY TO SIGN")
+TERMINAL_STATUSES=("SIGNED" "EXECUTED" "CANCELLED")
+VALID_STATUSES=( "${NON_TERMINAL_STATUSES[@]}" "${TERMINAL_STATUSES[@]}" )
errors=() # We collect all errors then print them at the end.
+# Name of a file to exclude from searching for non-terminal tasks.
+FOLDER_WITH_NO_TASKS="templates"
+# Name of a file in a task directiory that specifies that the task is a nested safe task.
+IF_THIS_ITS_NESTED="NestedSignFromJson.s.sol"
+
+single_tasks_to_simulate=()
+nested_tasks_to_simulate=()
+
# Function to check status and hyperlinks for a single file.
check_status_and_hyperlinks() {
local file_path=$1
@@ -42,6 +52,28 @@ check_status_and_hyperlinks() {
fi
}
+search_non_terminal_tasks(){
+ local directory
+ for file in $filtered_files; do
+ # Ensure it's a regular file.
+ if [[ -f "$file" ]]; then
+ # Read file content and search for any status in the NON_TERMINAL_STATUSES array.
+ for status in "${NON_TERMINAL_STATUSES[@]}"; do
+ if grep -q "$status" "$file"; then
+ directory=$(dirname "$file")
+ # Specify if a task is safe or nested.
+ if [[ -f "$directory/$IF_THIS_ITS_NESTED" ]]; then
+ nested_tasks_to_simulate+=("$file")
+ else
+ single_tasks_to_simulate+=("$file")
+ fi
+ break
+ fi
+ done
+ fi
+ done
+}
+
# Find README.md files for all tasks and process them.
files=$(find ./tasks -type f -path './tasks/*/*/README.md')
for file in $files; do
@@ -57,3 +89,15 @@ if [[ ${#errors[@]} -gt 0 ]]; then
else
echo "✅ All task statuses are valid"
fi
+
+# Excludes tasks defined in folder FOLDER_WITH_NO_TASKS
+filtered_files=$(echo "$files" | grep -v "/${FOLDER_WITH_NO_TASKS}/")
+search_non_terminal_tasks
+echo "Simulating single tasks..."
+for value in "${single_tasks_to_simulate[@]}"; do
+ echo "$value"
+done
+echo "Simulating nested tasks..."
+for value in "${nested_tasks_to_simulate[@]}"; do
+ echo "$value"
+done |
Thank you!
This is correct, for those it should always be something like # single.md
SIMULATE_WITHOUT_LEDGER=1 just \
--dotenv-path $(pwd)/.env \
--justfile ../../../single.just \
simulate
# nested.md
# for nested, first simulate as council
SIMULATE_WITHOUT_LEDGER=1 just \
--dotenv-path $(pwd)/.env \
--justfile ../../../nested.just \
simulate \
council 0
# then simulate as foundation
SIMULATE_WITHOUT_LEDGER=1 just \
--dotenv-path $(pwd)/.env \
--justfile ../../../nested.just \
simulate \
foundation 0 Do you mind opening a draft PR and we can continue discussions there? That would make it easier to stay organized :) For |
A draft PR is ready. |
Comment from @maurelian:
This is a good idea, since some tasks are dependent on previous tasks, and something we should think about how to best support in a follow up PR |
In
script/utils/check-task-statuses.sh
we define the following allowed task status:superchain-ops/script/utils/check-task-statuses.sh
Line 4 in 4a7d0f3
Of these, the SIGNED (for presigned pauses only), EXECUTED. and CANCELLED statuses are terminal, meaning once a task's status is changed to this value it should never change again.
When we add new tasks, the task author must manually remember to add them to CI (see
just_simulate_sc_rehearsal_1
as an example), but this is easy to forget. Instead, we should have CI do the following:tasks/*
directories.NestedSignFromJson
it's a nested safe task, otherwise it's a standard single safe)The reason we filter tasks by "tasks in a non-terminal status" instead of "statues that precede execution" is to be more robust: If a new task status is added, by default CI will try executing tasks with those status, and we can add that task status to the terminal status list if needed
The text was updated successfully, but these errors were encountered: