Skip to content

fix: fix skipped tests #1964

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

Merged
merged 3 commits into from
Feb 14, 2025
Merged

fix: fix skipped tests #1964

merged 3 commits into from
Feb 14, 2025

Conversation

philasmar
Copy link
Contributor

@philasmar philasmar commented Jan 31, 2025

Issue #, if available:
DOTNET-7945

Description of changes:

  • Moved common utilities to the Common project
  • Fixed previously skipped tests
  • PackagingTests unit tests were previously hanging due to the MSBUILD Target in the test tool project which was building and publishing RuntimeSupport during the test run. I changed this behavior to only build and publish when the TestTool project is being built. When the test is ran, the needed binaries are copied over to the NuGet package. This resolves the hanging.
  • To support the previous behavior, I created Targets for running all the tests in the CI as well as the unit and integ tests separately. GitHub CI and Release Pipeline will now trigger the build.proj instead of calling dotnet test directly.
  • In some tests, the executed Lambda function was writing to Standard Error which was causing dotnet msbuild to fail with a -1 exit code. I had to ignore Console.Error in those tests.
  • For integration tests, with the help of @GarrettBeatty, I moved the tests from testing an external project in the repo to only writing the function handler method and passing it to the Bootstrap similar to other tests.

Note: The tests are still flaky as there are other underlying issues such as us setting Environment Variables for tests which is global for all tests. However, this PR fixes the definitive failure of the tests that were skipped.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@philasmar philasmar marked this pull request as draft January 31, 2025 14:53
@philasmar philasmar added the Release Not Needed Add this label if a PR does not need to be released. label Jan 31, 2025
@philasmar philasmar force-pushed the asmarp/fix-skipped-tests branch 12 times, most recently from 1af68f8 to d13399a Compare February 7, 2025 13:56
@philasmar philasmar changed the title commit 1 fix: fix skipped tests Feb 7, 2025
@philasmar philasmar force-pushed the asmarp/fix-skipped-tests branch from 57c97e3 to e9f8869 Compare February 7, 2025 17:56
@philasmar philasmar marked this pull request as ready for review February 7, 2025 17:58
@philasmar philasmar force-pushed the asmarp/fix-skipped-tests branch from e9f8869 to dedaa12 Compare February 10, 2025 13:55
@philasmar philasmar force-pushed the asmarp/fix-skipped-tests branch from dedaa12 to 0af4817 Compare February 13, 2025 15:26
@philasmar philasmar force-pushed the asmarp/fix-skipped-tests branch from 80d2b84 to 243f389 Compare February 13, 2025 18:59
@philasmar philasmar merged commit 8f18bb2 into dev Feb 14, 2025
5 checks passed
philasmar added a commit that referenced this pull request Feb 14, 2025
philasmar added a commit that referenced this pull request Feb 14, 2025
fix: fix skipped tests (#1964) (Merging changes already pushed to `dev`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release Not Needed Add this label if a PR does not need to be released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants