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

Don't try to run tests from SIL.LCModel.Utils.Tests #420

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

jasonleenaylor
Copy link
Contributor

@jasonleenaylor jasonleenaylor commented Dec 3, 2024

This change is Reviewable

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.

2 participants