-
Notifications
You must be signed in to change notification settings - Fork 3
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
Conversation
Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
@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 |
step ids are strings
The log view (if you press open the full log not the custom execution one) stuff is here:
|
Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
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. Lines with an empty msg (a blank log line) are printed in json as well, e.g. this is due to |
…s json flag Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
Oh I see, you were referring to the
Is this what you notice when you debug this line: https://github.com/SanjulaGanepola/github-local-actions/pull/95/files#diff-6fc84a40da44590a80989de20f8247b011a4c4f759244213f9553adce87e2e68R427
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 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. |
I didn't expect to see
In the reconstructed readable log. I expected to see
or the following with a correctly colored job short notation
Yes, you have something similar but 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.
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? |
Can you give me an example workflow step to reproduce what you saw?
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:
Isn't this the expected result?
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.
In act, there indeed is no
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.
Skipping jobs and steps does work fine. However, I would like to show in the tree view when something is skipped. I was hoping |
Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
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
if (userOptions.includes(Option.Json)) { | ||
message = line; | ||
} |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you notice any issue here?
Maybe this was a regression for me, because I had --json
set previously without this change
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
Yes, I decided to switch
Can you please open a separate issue regarding this. I am considering adding a 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 |
My logic is similar to this one: (could be more complex)
|
…lete job Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
@ChristopherHX In order to get this PR merged, I made the following decisions (I am open to suggestions) in my latest commit so we can still create a release for now with this feature (until we get nektos/act#2552):
The below enhancements will be made in a different PR. The first bullet can be done without your act PR, but the other 2 bullets will have to wait until we get that act PR in.
Can you do a review of this PR once more to see if there are any issues. If nothing, I will create a release with it. Also, you mentioned previously you found issues with matrix jobs and reusable workflows. If these are issues with the extension, can you please open an issue so I can investigate it. Or are these issues in act itself? |
Would need to debug the following, do I have the correct version of your extension need to build from src anyway for debugging (this is nektos act)
|
eh Me always use the rerun feature always get breaking changes? |
No run from scratch didn't fix it... |
If this are matrices over matrices then it is an act issue as well, but reusable workflows are not correctly implemented and not part of my roadmap. I will create some issues that is an actual problem here. |
I would like Set up job to be removed if a setup job is logged by the cli tool. E.g no log entries for implicit |
Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
Signed-off-by: Sanjula Ganepola <sanjulagane@gmail.com>
All current changes I am testing with are pushed to this branch. As long as you pull and run the extension, you can debug with the latest stuff.
Let me know if you still have an issue with the restart option. If you are restarting really old history, it could be possible that you get an error but if you do the recent ones it shouldn't. Anything you start with the current changes should then be fine for future restarts. Let me know if you find any problem here.
For the sake of consistency with Let me know if this is now good to merge. |
yes, but if I download the vsix from ci I'm not 100% sure it was the current version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This has been a false positive and did apply to run from scratch as well, this issue is fixed now |
Yup the latest ci run should have the updated VISX. In the picture you sent, what is that first “test” step? Just wondering cuz I see the status is unknown. |
my Runner.Client tools is very verbose. This are pre queue log line, e.g. job level log. The other unknown state is workflow level log. |
You get there logs how my tool evaluates the job if and matrices, before knowing the matrix and name |
Can you share the workflow and full output for that execution? |
Well actually matrix strategy were on as well
I believe this is the same thing you could already have from my side
That the fake step test and the fake step the workflow name is because my tool doesn't emit step completion for them at this moment. This all happen after I decided to just reuse the timeline code of the webui for workflow and job level logging Your extension always has less information than my web server that manages everything in my project from 2021. My tool is able to operate on the local network across VM's, containers or even the internet. |
Changes
--json
flagRun Event
trigger separate tasks for each workflow so-W
flag is always usedFocus Task
for history tree item to focus on the associated running taskCurrently blocked by some missing JSONs in act itself: nektos/act#2551