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

Add support for job and step level execution status #95

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

SanjulaGanepola
Copy link
Owner

@SanjulaGanepola SanjulaGanepola commented Nov 30, 2024

Changes

  • Add job and step level execution status
  • Fix job ID instead of job name being displayed
  • Fix displayed output when user's select --json flag
  • Make Run Event trigger separate tasks for each workflow so -W flag is always used
  • Add right-click action to Focus Task for history tree item to focus on the associated running task

Currently blocked by some missing JSONs in act itself: nektos/act#2551

image

@SanjulaGanepola
Copy link
Owner Author

@ChristopherHX I made great progress on this PR today, but I am blocked in some aspects due to the issues described here: nektos/act#2551

@SanjulaGanepola SanjulaGanepola linked an issue Nov 30, 2024 that may be closed by this pull request
src/act.ts Outdated Show resolved Hide resolved
@ChristopherHX
Copy link
Contributor

ChristopherHX commented Nov 30, 2024

The log view (if you press open the full log not the custom execution one) stuff is here:

Signed-off-by: Sanjula Ganepola <[email protected]>
@ChristopherHX
Copy link
Contributor

There should not be any overflow that I know of. I tried with the following step below which prints a really long line with act and it appears to output correctly. Is this reproducible with act or just your cli shim?

I'm not sure what it is, but yes only my cli shim and that only in your extension.

I would assume some output buffering? Maybe is logrus flushing stdout more often than Console.WriteLine.
I consistently get some lines wrapped, but which ones that are affected is a bit random.

Lines with an empty msg (a blank log line) are printed in json as well, e.g. this is due to '' converted to false in an if as this is not an existence check

@SanjulaGanepola
Copy link
Owner Author

The log view (if you press open the full log not the custom execution one)

Oh I see, you were referring to the View job logs action in the official extension? At the moment, the extension will just open it as a log file and has some degree of colorization. This can definitly be improved.

image

I would assume some output buffering? Maybe is logrus flushing stdout more often than Console.WriteLine.
I consistently get some lines wrapped, but which ones that are affected is a bit random.

Is this what you notice when you debug this line: https://github.com/SanjulaGanepola/github-local-actions/pull/95/files#diff-6fc84a40da44590a80989de20f8247b011a4c4f759244213f9553adce87e2e68R427

Lines with an empty msg (a blank log line) are printed in json as well, e.g. this is due to '' converted to false in an if as this is not an existence check

If a line is empty, it will be filtered out here (https://github.com/SanjulaGanepola/github-local-actions/pull/95/files#diff-6fc84a40da44590a80989de20f8247b011a4c4f759244213f9553adce87e2e68R427). If however, the parsedMessage.msg is an empty string where parsedMessage is a JSON object of the parsed line, then it is expected for this to be shown as this is part of the actual output. Can you clarify the issue here?

Also, I have completed the original checklist in this PR and it is pretty much ready to merge. The only remaining items to be completed are:

For that last item, do you think it could be possible to implement easily? I would consider it higher priority than https://github.com/nektos/act/tree/draft-dump-as-json which we were discussing earlier.

@ChristopherHX
Copy link
Contributor

If however, the parsedMessage.msg is an empty string where parsedMessage is a JSON object of the parsed line, then it is expected for this to be shown as this is part of the actual output. Can you clarify the issue here?

I didn't expect to see

{"dryrun": false, ..., "msg":"",...}

In the reconstructed readable log.

I expected to see

[jobname]

or the following with a correctly colored job short notation

|

you were referring to the View job logs action in the official extension?

Yes, you have something similar but
yours did show ansi color code symbols instead of the color

Even if act strips the color in this case, my Runner.Client don't control what symbols are sent via the actions/runner jobs.

The view job logs command I copied removes ansi colors from the copyable log text and adds them as text decoration.

For that last item, do you think it could be possible to implement easily?

Coding this is easy, reviews are hard to get. Me with 6 open pr's without merge right am not fluent in this project and everything seems to become stale.

Are you sure pre steps have no results? I thought my github-act-runner from 2021 has no such problem, but yes it uses a low level logger that is not json output.

The setup and finish job steps doesn't actually exist in act

Does your code cannot handle skipped as stepResult?

@SanjulaGanepola
Copy link
Owner Author

I didn't expect to see
{"dryrun": false, ..., "msg":"",...}

Can you give me an example workflow step to reproduce what you saw?

I expected to see
[jobname]

When I try with a step like this:

      - name: Test empty output
        run: |
          echo "Line with content"
          echo ""
          echo "Line with more content"

The output using this PR is:

[Build Application/Build] ⭐ Run Main Test empty output
[Build Application/Build]   🐳  docker exec cmd=[bash -e /var/run/act/workflow/1] user= workdir=
[Build Application/Build] Line with content
[Build Application/Build]
[Build Application/Build] Line with more content
[Build Application/Build]   ✅  Success - Main Test empty output

Isn't this the expected result?

Coding this is easy, reviews are hard to get. Me with 6 open pr's without merge right am not fluent in this project and everything seems to become stale.

If we can get a PR open for this specific change, I can try to follow up with Casey to see if he will merge it as it is critical for this feature.

Are you sure pre steps have no results? I thought my github-act-runner from 2021 has no such problem, but yes it uses a low level logger that is not json output.

In act, there indeed is no stepResult for the last pre step even though there is one of the post step.

The setup and finish job steps doesn't actually exist in act

This will need to be added to act in order to support this or we will just have to assume them as success which does not make sense to do.

Does your code cannot handle skipped as stepResult?

Skipping jobs and steps does work fine. However, I would like to show in the tree view when something is skipped. I was hoping act could log when a job or step was skipped so I can parse this information.

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Dec 1, 2024

Exception has occurred: TypeError: commandArgs.options.join is not a function

Hmm, just rebased now commandArgs.options is a string for me not array..., not that all tested this extension get a problem?

Applies only to restart old jobs

src/act.ts Outdated
Comment on lines 525 to 527
if (userOptions.includes(Option.Json)) {
message = line;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This forces me to see json in both console and log view

Copy link
Owner Author

Choose a reason for hiding this comment

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

Even though we technically always now use the --json option, I still want to show the output to the user as normal. However, if user decides to add the --json flag from the Options tree item in the Settings view, we should show the output as json. If the user does not add it, the output should appear as normal. Did you notice any issue here?

@ChristopherHX
Copy link
Contributor

I did saw surprising stuff in matrix jobs, but not sure if act has the same thing.

Total confusion is probably via reusable workflows that are barely supported

…ept-empty-msg

buffer last line without new line end
@SanjulaGanepola
Copy link
Owner Author

Applies only to restart old jobs

Yes, I decided to switch options in CommandArgs to an array. This will cause an issue for restarting old jobs that have options as a string but I think this should be okay.

I did saw surprising stuff in matrix jobs, but not sure if act has the same thing.

Total confusion is probably via reusable workflows that are barely supported

Can you please open a separate issue regarding this. I am considering adding a Matrix tree item heading to the Settings view to configure matrices more easily as mentioned here. What do you think about this?

Since we are unsure when nektos/act#2551 will be resolved, what do you think we should do in the meantime to get this feature out? Should we simply mark them as Success or keep them in the Unknown state?

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Dec 2, 2024

Since we are unsure when nektos/act#2551 will be resolved, what do you think we should do in the meantime to get this feature out? Should we simply mark them as Success or keep them in the Unknown state?

My logic is similar to this one: (could be more complex)

  • if setup job is followed by a new step => success
  • if act exists non zero even if it says job success fails all unknown steps problem with > 1 job that my github-act-runner never has to deal with.
  • if a main steps ignores a step known from the yaml, set status skipped, if you see a log of step after it
  • complete job doesn't exists for me as step if it logs something it's part of the last step

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.

Add job and step level execution state in History view
2 participants