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

FAKE5 Targets always complete as "Failed" #1929

Closed
BlythMeister opened this issue May 14, 2018 · 16 comments
Closed

FAKE5 Targets always complete as "Failed" #1929

BlythMeister opened this issue May 14, 2018 · 16 comments

Comments

@BlythMeister
Copy link
Contributor

Description

The logging of a target always finished with "Finished (Failed) 'TargetName' in

It would appear the the default state on tracing is Failed, and this is never set to success in the target helper if it runs to completion.

@matthid
Copy link
Member

matthid commented May 14, 2018

Can you elaborate a bit I don't get it. Maybe a screenshot/sample/command line you use would help?

@BlythMeister
Copy link
Contributor Author

Sure, our scripts are open source, so if you look at this repo https://github.com/15below/Build.Tools/tree/NewTools
Main.fsx is the entry point, and at the moment the targets are all empty.
When these run you always get the following logs.

Started
No description
Finished

@BlythMeister
Copy link
Contributor Author

Looking at the logs for https://ci.appveyor.com/project/matthid/fake-6w516/build/1.0.1571 it would seem that you get (Success).

I wonder if there is something in the code for the target which sets the status...
However, I think If you do the simple hello world from https://fake.build/fake-gettingstarted.html you will get that symptom

@sliepie
Copy link
Contributor

sliepie commented May 14, 2018

I've seen the same after upgrading to rc012, while running the following snippet:

Target.create "Bootstrap" (fun _ ->
    Trace.setBuildNumber (sprintf "%s-%s" buildNumber branchName)
)

Output:
image

@matthid
Copy link
Member

matthid commented May 14, 2018

So it seems to be a minor annoyance - fake printing the wrong stuff, correct?

@matthid
Copy link
Member

matthid commented May 14, 2018

I think this is related to 9d29f00. Idea is to report "tasks" as failed when _.MarkSuccess() is not called (which indicates an exception). Seems like I have missed some places there.

@sliepie
Copy link
Contributor

sliepie commented May 14, 2018

@matthid correct it is a minor issue, as build passes fine.

@kblohm
Copy link
Contributor

kblohm commented May 14, 2018

Instead of using a disposable, cant u just wrap everything in a function that forces u to return succes/failure and handle the try catch finally inside whatever calls the function. That way you cant forget to call MarkSucces(). I am pretty sure most people will just forget to call it.

As in Trace.traceTask "MyOperation" "Description" <| fun () -> TagStatus.Success
That would of course be a breaking change though.

@matthid
Copy link
Member

matthid commented May 14, 2018

That would of course be a breaking change though.

Yes we could add that as additional API, because we no longer do breaking changes at this point.

@matthid matthid mentioned this issue May 14, 2018
5 tasks
@matthid
Copy link
Member

matthid commented May 14, 2018

Actually I failed two times because the default should be Warning instead of Error. Now Warning isn't used at all :)

@BlythMeister
Copy link
Contributor Author

Can someone please explain how I should be writing the target then to use mark success. As I'm a bit confused and can't see in docs

@matthid
Copy link
Member

matthid commented May 15, 2018

Sorry I didn’t make that clear. I think it is an internal bug. Just tried to clarify the issue and reasoning if someone wants to step in

@BlythMeister
Copy link
Contributor Author

But if I have a target which I only do private stuff in, I don't want it to report warning if it was ok.

Imo, if it has an error thrown, it's error.
If it doesn't, its success...

@matthid
Copy link
Member

matthid commented May 15, 2018

Ok but it’s not so easy to detect. But yeah I guess we just need to fix this bug and it will be fine again

@BlythMeister
Copy link
Contributor Author

Psudo code:

Try
execute target
Mark success
With
Mark failure
Throw

Might not even need the with as default is failure

@matthid
Copy link
Member

matthid commented May 17, 2018

I found the problem and it is fixed with rc13

@matthid matthid closed this as completed May 17, 2018
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

No branches or pull requests

4 participants