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

Script to convert TAP to GHA workflow commands #1052

Merged
merged 3 commits into from
May 28, 2021
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented May 27, 2021

I plan to wire this into the github actions workflow, to make it easier to see
what broke.

I plan to wire this into the github actions workflow, to make it easier to see
what broke.
@richvdh richvdh requested a review from a team May 27, 2021 12:20
my $number = $result->number;
my $description = $result->description;

print "${RED}FAILURE:$RESET_FG #$number: $description\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should look for GITHUB_ACTIONS being set in the environment, and if so, log the error in a format that it will detect?

E.g., print ("::error::#$number: $description\n");

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh I looked for a document like that, and failed to find one.

I think it makes sense for this to be GHA-specific, rather that having magic based on the environment. I'll have a go at this.

@richvdh richvdh changed the title Script to convert TAP to text Script to convert TAP to GHA workflow commands May 28, 2021
@richvdh richvdh requested a review from callahad May 28, 2021 11:08
@callahad
Copy link
Contributor

Output looks good to me. Wonder if this should live here or in Synapse where it's easier to iterate on, but 🤷.

Wonder if the grouping is strictly necessary, but it doesn't hurt.

I have a stripped down PR running at matrix-org/synapse#10090 which I'll re-start until it naturally hits a failure just so we can have a "real" example of how this script behaves.

@anoadragon453
Copy link
Member

Wonder if this should live here or in Synapse where it's easier to iterate on, but shrug.

It can be useful for other homeserver implementations, so should probably live in the Sytest repo.

@callahad
Copy link
Contributor

Actually, one requested change: could this return a non-0 exit code when failures are detected?

@callahad
Copy link
Contributor

callahad commented May 28, 2021

For context, this is what the output from this script looks like 🎉

Screen Shot 2021-05-28 at 13 51 57

Screen Shot 2021-05-28 at 13 51 36

@richvdh
Copy link
Member Author

richvdh commented May 28, 2021

Wonder if this should live here or in Synapse where it's easier to iterate on, but shrug.

It can be useful for other homeserver implementations, so should probably live in the Sytest repo.

Yup absolutely this. Likewise, I can't see other test frameworks being silly enough to write TAP output.

@richvdh
Copy link
Member Author

richvdh commented May 28, 2021

Actually, one requested change: could this return a non-0 exit code when failures are detected?

your wish is my command. If you're happy with this can you mash the merge button?

@richvdh richvdh requested a review from callahad May 28, 2021 13:43
@callahad callahad merged commit 07aaafd into develop May 28, 2021
@callahad callahad deleted the rav/tap_to_text branch May 28, 2021 14:28
richvdh added a commit to matrix-org/synapse that referenced this pull request Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants