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

traceImportant produces a normal message in TeamCity #2108

Closed
otto-gebb opened this issue Sep 25, 2018 · 20 comments
Closed

traceImportant produces a normal message in TeamCity #2108

otto-gebb opened this issue Sep 25, 2018 · 20 comments

Comments

@otto-gebb
Copy link
Contributor

otto-gebb commented Sep 25, 2018

traceImportant produces a normal message in TeamCity, although it should produce a warning.

Repro steps

  1. Set TEAMCITY_VERSION environment variable to 1.
  2. Run a FAKE script with a traceImportant call in it.

Expected behavior

The message is logged as

##teamcity[message text='your_message_here' status='WARNING']

Actual behavior

The message is logged as

your_message_here

Known workarounds

Use this function instead of traceImportant:

let logWarning str =
  let inline escape text =
    text
    |> String.replace "|" "||"
    |> String.replace "'" "|'"
    |> String.replace "\n" "|n"
    |> String.replace "\r" "|r"
    |> String.replace "[" "|["
    |> String.replace "]" "|]"
  if isLocalBuild
  then Trace.traceImportant str
  else Trace.logfn "##teamcity[message text='%s' status='WARNING']" <| escape str

Related information

@BlythMeister
Copy link
Contributor

To extend this, error also doesn't work as status should be error.

@matthid
Copy link
Member

matthid commented Sep 26, 2018

@BlythMeister So when you (or any api you use) uses traceError you expect the build to fail?

@BlythMeister
Copy link
Contributor

No, but reporting a message as an error message doesn't make build fail (I don't think)

There is another issue that when a target ends with a failure result, the TeamCity listener should post an build failure with the close block error message. (I think)

@BlythMeister
Copy link
Contributor

As far as I know, in TeamCity 'ERROR' != 'FAILURE'

@matthid
Copy link
Member

matthid commented Sep 26, 2018

@BlythMeister Can you verified that?

@otto-gebb
Copy link
Contributor Author

otto-gebb commented Sep 26, 2018

@BlythMeister A build definition has a setting in the "Failure conditions" that looks like "Fail build when errors are logged".

@BlythMeister
Copy link
Contributor

Ahh yeah, I never have that enabled 😂

@matthid
Copy link
Member

matthid commented Sep 26, 2018

The reason why I asked is because we tried to change stuff there in there past and usually people start to complain that there builds are failing (even when they should fix their builds)

@matthid
Copy link
Member

matthid commented Sep 26, 2018

Or put differently, when we decide to do it I'll probably close those issues with "by-design" or have a workaround ready ;)
We are not reverting afterwards like we did in the past (people can lock the old versions of the module), but we should be sure we are doing the right thing.

@matthid
Copy link
Member

matthid commented Sep 26, 2018

We could for example add documentation for the TeamCity option if that already exists.

otto-gebb added a commit to otto-gebb/FAKE that referenced this issue Sep 26, 2018
@otto-gebb
Copy link
Contributor Author

We can start by changing just the behavior of traceImportant, that would generate a warning. Warnings do not fail the build. Can we?

@matthid
Copy link
Member

matthid commented Sep 26, 2018

@otto-gebb I just want to discuss and decide this once and for all, we changed to policy countless times in the past, reverted releases/prs and I want this to end :)

@otto-gebb
Copy link
Contributor Author

otto-gebb commented Sep 26, 2018

@matthid I get it. In my opinion traceError should produce an error in TeamCity. If this breaks the build for someone, they should turn off the "fail build on error logged" setting. Still, logging of warning may be considered to be an independent concern.

@BlythMeister
Copy link
Contributor

Reporting Messages For Build Log

You can report messages for a build log in the following way:

##teamcity[message text='' errorDetails='' status='']

where:

The status attribute may take following values: NORMAL, WARNING, FAILURE, ERROR. The default value is NORMAL.

The errorDetails attribute is used only if status is ERROR, in other cases it is ignored.

This message fails the build in case its status is ERROR and "Fail build if an error message is logged by build runner" box is checked on the Build Failure Conditions page of a build configuration. For example:

@BlythMeister
Copy link
Contributor

Quote fail .. but that is from the TeamCity docs on status messages.

@BlythMeister
Copy link
Contributor

@matthid I'm happy to raise a pr to make fake work to how the TeamCity docs state.
We can then quote TeamCity docs as a "works as designed"

@matthid
Copy link
Member

matthid commented Sep 26, 2018

@BlythMeister Yes that would be very welcome, feel free to update the docs (=api docs for the teamcity module similar to the TeamFoundation one) as well (just link to official docs)

@BlythMeister
Copy link
Contributor

@matthid this would extend on @otto-gebb pr to include trace error and a "block" ending with a failure.
@otto-gebb, I'll base my branch off of yours and raise it tomorrow. 👍

@otto-gebb
Copy link
Contributor Author

Currently this functionality is broken again, except it's even worse: traceImportant produces no message at all. @matthid should I create a separate bug?

@matthid
Copy link
Member

matthid commented Feb 15, 2019

Yes please, you can link back here. Also I'd love to get help on this....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants