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

${AssemblyVersion}: add type (File, Assembly, Informational) option #2487

Merged
merged 16 commits into from
Jan 4, 2018
Merged

${AssemblyVersion}: add type (File, Assembly, Informational) option #2487

merged 16 commits into from
Jan 4, 2018

Conversation

alexangas
Copy link
Contributor

@304NotModified
Copy link
Member

304NotModified commented Jan 3, 2018

The build server says:

'AssemblyVersionLayoutRenderer.GetVersion()': not all code paths return a value [C:\projects\nlog\src\NLog\NLog.csproj]

Could you please check that?
See https://github.com/NLog/NLog/pull/2487/files#r159549927

{
if (!string.IsNullOrEmpty(assemblyName))
var assemblyName = GetAssemblyName();
assemblyName.Version.ToString();
Copy link
Member

Choose a reason for hiding this comment

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

missing return ;)

@304NotModified 304NotModified added this to the 4.5 rc4 milestone Jan 3, 2018
Copy link
Member

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

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

nice, looks good!


private static Assembly GenerateTestAssembly()
{
const string code = @"
Copy link
Member

Choose a reason for hiding this comment

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

nice!

@304NotModified
Copy link
Member

Unfortunately, it fails on Travis

.> NLog.UnitTests.LayoutRenderers.AssemblyVersionTests.AssemblyVersionTypeTest(type: Assembly, expected: "1.1.1.1") [FAIL]
System.InvalidOperationException : Unable to find field m_entryAssembly

Any idea why? See https://travis-ci.org/NLog/NLog/builds/324836802

…dling of Roslyn-generated version type unit test when we have issues of entry assembly.
@codecov
Copy link

codecov bot commented Jan 4, 2018

Codecov Report

Merging #2487 into master will decrease coverage by 1%.
The diff coverage is 70%.

@@           Coverage Diff           @@
##           master   #2487    +/-   ##
=======================================
- Coverage      82%     82%    -1%     
=======================================
  Files         322     322            
  Lines       24215   23219   -996     
  Branches     3225    2888   -337     
=======================================
- Hits        19912   18939   -973     
+ Misses       3542    3511    -31     
- Partials      761     769     +8

@alexangas
Copy link
Contributor Author

The issue appears to be the way that xunit handles app domains, described in microsoft/vstest#649. This means that calls to Assembly.GetEntryAssembly() may return null (as can happen in existing EntryAssemblyVersionTest) or attempts to force set it may fail (as can happen in new AssemblyVersionTypeTest).

There may be a way to get around this by changing the xunit config value xunit.appDomain, but it looks like this may affect all tests and cause adverse effects.

So in these most recent pushes I have taken the same approach as the existing EntryAssemblyVersionTest when this wasn't working where it effectively skips the test if there are problems. It also attempts to log that.

@304NotModified
Copy link
Member

So in these most recent pushes I have taken the same approach as the existing EntryAssemblyVersionTest when this wasn't working where it effectively skips the test if there are problems. It also attempts to log that

seems to work great!

Thanks, looks good!

Could you please update the wiki and add "Introduced in NLog 4.5"? Thanks in advance!

@304NotModified 304NotModified changed the title Adds version types to AssemblyVersion layout renderer ${AssemblyVersion}: add type (File, Assembly, Informational) option Jan 4, 2018
@304NotModified 304NotModified merged commit 5747300 into NLog:master Jan 4, 2018
@alexangas alexangas deleted the feature/assembly-renderer-add-version-types branch January 5, 2018 09:13
@alexangas
Copy link
Contributor Author

@304NotModified
Copy link
Member

thanks! also for adding examples! 👍 👍

@snakefoot snakefoot mentioned this pull request Jan 14, 2018
@snakefoot snakefoot modified the milestones: 4.5 rc4, 4.5 Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants