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 Additional Information to TeamCityBuildInfo #3113

Merged
merged 4 commits into from
Feb 17, 2021

Conversation

BlythMeister
Copy link
Contributor

@BlythMeister BlythMeister commented Feb 10, 2021

Adding support in the TeamCityBuildInfo to load the properties XML files as a key/value data store.
Files loaded as a Lazy to prevent IO when properties are not required & to also then cache the results if subsequent queries are to properties are required.

Also exposing the teamcity branch name.

NOTE: no test of loading the XML files have been added as i am unsure about the rules/convention for this sort of test for cake.

Fixes #2967

@dnfadmin
Copy link

dnfadmin commented Feb 10, 2021

CLA assistant check
All CLA requirements met.

@dnfadmin
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ BlythMeister sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Member

@devlead devlead left a comment

Choose a reason for hiding this comment

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

NOTE: no test of loading the XML files have been added as i am unsure about the rules/convention for this sort of test for cake.

Usually, we add sample XML to resources so we can test functionality works on known good data and keep that working overtime.

One example you can look at is XML peek tests

Resources

public static string XmlPeek_Xml_With_Namespace {
get {
return ResourceManager.GetString("XmlPeek_Xml_With_Namespace", resourceCulture);
}
}

<data name="XmlPeek_Xml_With_Namespace" xml:space="preserve">
<value>&lt;?xml version="1.0" encoding="utf-8"?&gt;
&lt;Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"&gt;
&lt;PropertyGroup&gt;
&lt;WebPublishMethod&gt;FileSystem&lt;/WebPublishMethod&gt;
&lt;LastUsedBuildConfiguration&gt;DeploymentTemplate&lt;/LastUsedBuildConfiguration&gt;
&lt;LastUsedPlatform&gt;Any CPU&lt;/LastUsedPlatform&gt;
&lt;SiteUrlToLaunchAfterPublish /&gt;
&lt;LaunchSiteAfterPublish&gt;True&lt;/LaunchSiteAfterPublish&gt;
&lt;ExcludeApp_Data&gt;False&lt;/ExcludeApp_Data&gt;
&lt;publishUrl&gt;C:\Deployment\DeploymentTemplate\WebApi&lt;/publishUrl&gt;
&lt;DeleteExistingFiles&gt;False&lt;/DeleteExistingFiles&gt;
&lt;/PropertyGroup&gt;
&lt;/Project&gt;</value>
</data>

How it can be accessed/used in tests.

[Fact]
public void Should_Get_Element_Value_With_Namespace()
{
// Given
var fixture = new XmlPeekAliasesFixture();
fixture.SetContent(Properties.Resources.XmlPeek_Xml_With_Namespace);
fixture.Settings.Namespaces.Add("msbuild", "http://schemas.microsoft.com/developer/msbuild/2003");
// When
var result = fixture.Peek("/msbuild:Project/msbuild:PropertyGroup/msbuild:publishUrl");
// Then
Assert.Equal("C:\\Deployment\\DeploymentTemplate\\WebApi", result);
}

@BlythMeister
Copy link
Contributor Author

Perfect, I shall add a test with some sample data.

Thanks for the quick review

@BlythMeister
Copy link
Contributor Author

@devlead I have added tests now.

This did require me to update some other bits and other tests to support passing the IFileSystem through.
Other good news...the tests showed an issue with my XPath :D

@devlead
Copy link
Member

devlead commented Feb 13, 2021

@BlythMeister starting to look good to me 👍

There's a few merge conflicts because older PRs merged, could you please rebase against latest develop branch and take a look at addressing them?

Also is there an issue related to this PR? If not could you please add one? Normally we require issues before PR for buying, but we also use them for release notes and blog post generations.

@BlythMeister
Copy link
Contributor Author

Ahh sure.

I'll rebase on monday and will raise an issue and tag it to this pr in the description 👍

@BlythMeister
Copy link
Contributor Author

Turns out someone had raised an issue...I now raised a duplicate 🤦

@augustoproiete
Copy link
Member

@BlythMeister @devlead PR LGTM, but wanted to run something by you before it's too late to make changes (if any needed) to the TeamCityBuildInfo public API:

TeamCity currently generates 6 (six) files per build, 3 (three) different pairs / sets or properties (.properties and .properties.xml equivalent):

1. TeamCity configuration parameters for build

  • teamcity.build(...).properties and teamcity.build(...).properties.xml

2. TeamCity build properties without 'system.' prefix

  • teamcity.config(...).properties and teamcity.config(...).properties.xml

3. TeamCity build runner parameters

  • teamcity.runner(...).properties and teamcity.runner(...).properties.xml

This PR is implementing number (1) above and exposing the values TeamCity.Build.Properties, which is fine.

However, once we implement items (2) and (3), how should we make these properties available?

One option could be completely separate properties, e.g. TeamCity.Config.Properties and TeamCity.Runner.Properties in which case doesn't affect this PR.

Another option would be to merge all three files into TeamCity.Build.Properties but there are duplicate properties across the files, so we'd need to decide how to handle those.

One other option, would be to rename the property in this PR to be something more specific to the file it reads from e.g. TeamCity.Build.BuildProperties, and later use TeamCity.Build.ConfigProperties and TeamCity.Build.RunnerProperties for consistency.

What do you think?

@BlythMeister
Copy link
Contributor Author

I think renaming the properties makes good sense.

I would be happy to include the other files in this PR aswell if you think it would be useful?

I am already reading file 2 in order to find the path for file 1, so adding file 3 in and exposing file 2 rather than discarding it is pretty trivial and can implement them all with lazy, so files only read when needed.

I'm not working until Monday, but will sort conflicts and and the last file in then 👍

@augustoproiete
Copy link
Member

Sounds great @BlythMeister. I didn't want to augment the scope of your PR, but since you're offering I'll take it 😄

In terms of the API, I'm fine with 3 separate dictionaries with different names, but let's see if we can get a few more eyes on this PR from the @cake-build/cake-team in case they have other ideas

@BlythMeister
Copy link
Contributor Author

As a 1st pr and my 1st use of cake. So far the community have been great. I'm more than happy to give back where I can 🙂

@BlythMeister
Copy link
Contributor Author

@augustoproiete @devlead I have rebased this on develop & resolved the conflicts.
I have also now included all 3 of the properties files as part of the BuildInfo

@augustoproiete
Copy link
Member

End-to-end test results running on TeamCity 2020.2.2:

Feature Comments
TeamCity.Environment.Build.BranchName Branch name read from TeamCity as expected
TeamCity.Environment.Build.VcsBranchName VCS branch name read from TeamCity as expected
Properties from teamcity.build.properties.xml All build properties read from TeamCity as expected
Properties from teamcity.config.properties.xml All config properties from TeamCity as expected
Properties from teamcity.runner.properties.xml All runner properties from TeamCity as expected

TeamCity generated files

  • teamcity.build.properties.xml (content sampled)
<?xml version="1.0" encoding="utf-8" standalone="no"?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
<comment>TeamCity build properties without 'system.' prefix</comment>
<entry key="agent.home.dir">D:\TeamCity\buildAgent01</entry>
<entry key="agent.name">AUGUSTO-TC-01</entry>
<entry key="teamcity.version">2020.2.2 (build 85899)</entry>
</properties>

-teamcity.config.properties.xml (content sampled)

<?xml version="1.0" encoding="utf-8" standalone="no"?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
<comment>TeamCity configuration parameters for build with id 214913</comment>
<entry key="build.triggeredBy">C. Augusto Proiete</entry>
<entry key="build.triggeredBy.username">augustoproiete</entry>
<entry key="DotNetCLI_Path">C:\Program Files\dotnet\dotnet.exe</entry>
</properties>
  • teamcity.runner.properties.xml (content sampled)
<?xml version="1.0" encoding="utf-8" standalone="no"?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
<comment>TeamCity build runner parameters</comment>
<entry key="teamcity.build.checkoutDir">D:\agent01\w\ad0d02f87a570f9d</entry>
<entry key="teamcity.fail.exit.code">true</entry>
<entry key="teamcity.step.mode">default</entry>
<entry key="use.custom.script">true</entry>
</properties>

Test script

  • teamcity.cake
Information("TeamCity.Environment.Build.BranchName: {0}", TeamCity.Environment.Build.BranchName);
Information("TeamCity.Environment.Build.VcsBranchName: {0}", TeamCity.Environment.Build.VcsBranchName);

Information("");
Information("---");
Information("");

Information("TeamCity.Environment.Build.BuildProperties:");

foreach (var p in TeamCity.Environment.Build.BuildProperties)
{
    Information("  - {0}={1}", p.Key, p.Value);
}

Information("");
Information("---");
Information("");

Information("TeamCity.Environment.Build.ConfigProperties:");

foreach (var p in TeamCity.Environment.Build.ConfigProperties)
{
    Information("  - {0}={1}", p.Key, p.Value);
}

Information("");
Information("---");
Information("");

Information("TeamCity.Environment.Build.RunnerProperties:");

foreach (var p in TeamCity.Environment.Build.RunnerProperties)
{
    Information("  - {0}={1}", p.Key, p.Value);
}

Build log:

  • teamcity-build.log (content sampled)
[Step 1/1] TeamCity.Environment.Build.BranchName: develop
[Step 1/1] TeamCity.Environment.Build.VcsBranchName: refs/heads/develop
[Step 1/1]
[Step 1/1] ---
[Step 1/1]
[Step 1/1] TeamCity.Environment.Build.BuildProperties:
[Step 1/1]   - agent.home.dir=D:\TeamCity\buildAgent01
[Step 1/1]   - agent.name=AUGUSTO-TC-01
[Step 1/1]   - teamcity.version=2020.2.2 (build 85899)
[Step 1/1]
[Step 1/1] ---
[Step 1/1]
[Step 1/1] TeamCity.Environment.Build.ConfigProperties:
[Step 1/1]   - build.triggeredBy=C. Augusto Proiete
[Step 1/1]   - build.triggeredBy.username=augustoproiete
[Step 1/1]   - DotNetCLI_Path=C:\Program Files\dotnet\dotnet.exe
[Step 1/1]
[Step 1/1] ---
[Step 1/1]
[Step 1/1] TeamCity.Environment.Build.RunnerProperties:
[Step 1/1]   - teamcity.build.checkoutDir=D:\agent01\w\ad0d02f87a570f9d
[Step 1/1]   - teamcity.fail.exit.code=true
[Step 1/1]   - teamcity.step.mode=default
[Step 1/1]   - use.custom.script=true
[Step 1/1] Process exited with code 0
[23:47:05] : Publishing internal artifacts (1s)
[23:47:06] :     [Publishing internal artifacts] Publishing 1 file using [WebPublisher]
[23:47:06] :     [Publishing internal artifacts] Publishing 1 file using [ArtifactsCachePublisher]
[23:47:06] : Build finished

Copy link
Member

@augustoproiete augustoproiete left a comment

Choose a reason for hiding this comment

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

LGTM!

@augustoproiete augustoproiete merged commit 362db0e into cake-build:develop Feb 17, 2021
@augustoproiete
Copy link
Member

@BlythMeister your changes have been merged, thanks for your contribution 👍

@BlythMeister BlythMeister deleted the TeamCity branch February 18, 2021 09:42
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.

Expose TeamCity build properties dictionary via TeamCityBuildInfo
4 participants