Skip to content

Conversation

@jasonleenaylor
Copy link
Contributor

@jasonleenaylor jasonleenaylor commented Dec 3, 2024

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Dec 3, 2024

Test Results

554 tests   - 327   554 ✅  - 213   3m 25s ⏱️ -1s
  6 suites  - 102     0 💤  -  25 
  6 files   +  5     0 ❌  -   5 

Results for commit f8b20bb. ± Comparison against base commit 2dcbbb7.

This pull request removes 881 and adds 554 tests. Note that renamed tests count towards both.
FLEx_ChorusPluginTests.Infrastructure.ActionHandlers.ViewNotesActionHandlerTests.AdjustConflictHtml_FixesChecksums
FLEx_ChorusPluginTests.Infrastructure.ActionHandlers.ViewNotesActionHandlerTests.AdjustConflictHtml_ReplacesDatabaseCurrent
FLEx_ChorusPluginTests.Infrastructure.ActionHandlers.ViewNotesActionHandlerTests.AdjustConflictHtml_ReplacesWsRuns
FLEx_ChorusPluginTests.Integration.MergeIntegrationTests.DictConfigMerge_DifferentUpgradePathKeepsFileFormat
FLEx_ChorusPluginTests.Integration.MergeIntegrationTests.EnsureDictionaryConfigsUseDictionaryStrategy
FLEx_ChorusPluginTests.Integration.MergeIntegrationTests.EnsureRightPersonMadeChanges
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests.NewCommentOnLF
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests.NothingNew
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests.StatusChangeOnLD("")
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests.StatusChangeOnLD("c4f4df11-8dda-418e-8124-66406d67a2d1")
…
FLEx_ChorusPluginTests.Infrastructure.ActionHandlers.ViewNotesActionHandlerTests ‑ AdjustConflictHtml_FixesChecksums
FLEx_ChorusPluginTests.Infrastructure.ActionHandlers.ViewNotesActionHandlerTests ‑ AdjustConflictHtml_ReplacesDatabaseCurrent
FLEx_ChorusPluginTests.Infrastructure.ActionHandlers.ViewNotesActionHandlerTests ‑ AdjustConflictHtml_ReplacesWsRuns
FLEx_ChorusPluginTests.Integration.MergeIntegrationTests ‑ DictConfigMerge_DifferentUpgradePathKeepsFileFormat
FLEx_ChorusPluginTests.Integration.MergeIntegrationTests ‑ EnsureDictionaryConfigsUseDictionaryStrategy
FLEx_ChorusPluginTests.Integration.MergeIntegrationTests ‑ EnsureRightPersonMadeChanges
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests ‑ NewCommentOnLF
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests ‑ NothingNew
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests ‑ StatusChangeOnLD("")
LfMergeBridgeTests.LanguageForgeGetChorusNotesActionHandlerTests ‑ StatusChangeOnLD("c4f4df11-8dda-418e-8124-66406d67a2d1")
…

♻️ This comment has been updated with latest results.

* Exclude the SIL.LCModel.Utils.Tests.dll since that is a utility dll
  that has tests in it
Copy link
Contributor

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

looks good to me, I left a suggestion, but I'm not super sure if it'll work so feel free to discard it. I tried it and got an error I didn't really understand, but I'm not sure if that's just my environment.

OutputXmlFile="$(RootDir)/output/$(Configuration)/$(TargetFramework)/TestResults.xml"
TeamCity="$(TeamCity)"/>
<!-- Loop over each TestAssembly and execute dotnet test -->
<Exec Command="dotnet test --no-build --verbosity detailed --logger &quot;trx;LogFileName=%(TestAssemblies.Filename)TestResults.trx&quot; --filter &quot;TestCategory!=$(ExtraExcludeCategories)&quot; &quot;%(TestAssemblies.FullPath)&quot;"
Copy link
Contributor

Choose a reason for hiding this comment

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

escaping stuff sucks, could you do this instead?

Suggested change
<Exec Command="dotnet test --no-build --verbosity detailed --logger &quot;trx;LogFileName=%(TestAssemblies.Filename)TestResults.trx&quot; --filter &quot;TestCategory!=$(ExtraExcludeCategories)&quot; &quot;%(TestAssemblies.FullPath)&quot;"
<PropertyGroup>
<_RunTestsCommand>dotnet test --no-build --verbosity detailed --logger "trx;LogFileName=%(TestAssemblies.Filename)TestResults.trx" --filter "TestCategory!=$(ExtraExcludeCategories)" "%(TestAssemblies.FullPath)"</_RunTestsCommand>
</PropertyGroup>
<!-- Loop over each TestAssembly and execute dotnet test -->
<Exec Command="$(_RunTestsCommand)"

I'm not too familiar with msbuild so I'm not sure if that would work but it would be much more maintinable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, escaping stuff isn't my favorite, but in this case I think it is necessary.
Your suggestion wouldn't work because it would defeat the batching. Actually, the error you got is probably because the metadata expansion isn't valid in a PropertyGroup.
https://learn.microsoft.com/en-us/visualstudio/msbuild/item-metadata-in-task-batching?view=vs-2022

@jasonleenaylor jasonleenaylor merged commit a80711d into develop Dec 11, 2024
9 checks passed
@jasonleenaylor jasonleenaylor deleted the chore/excludeLCMTests branch December 11, 2024 16:47
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.

3 participants