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

Consolidate emitted IL for-loop tests #17128

Merged
merged 7 commits into from
May 13, 2024

Conversation

brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented May 9, 2024

Description

As noted in #17040:

In a followup PR, I may consolidate the copied "realsig±" test source files using a technique similar to that used in #16795.

The test sources and baselines for these were duplicated in #15484 to test the realsig feature, but that meant that the test sources needed to be kept in sync by hand. This change reconsolidates the test sources by using the approach from #16795 instead: it reuses the same test source files and just adds duplicate tests to check the emitted IL with realsig- and realsig+.

A few realsig- baseline files are updated as well, since it looks like some tests in /EmittedIL/ForLoop/RealInternalSignatureOff/ForLoop.fs did not have |> withLangVersionPreview, while their equivalents in /EmittedIL/ForLoop/RealInternalSignatureOn/ForLoop.fs did (proof that requiring manual synchronization is bad :).

For reviewers

The most important files:

  • Removed:
    • tests/FSharp.Compiler.ComponentTests/EmittedIL/ForLoop/RealInternalSignatureOff/ForLoop.fs
    • tests/FSharp.Compiler.ComponentTests/EmittedIL/ForLoop/RealInternalSignatureOn/ForLoop.fs
  • Added:
    • tests/FSharp.Compiler.ComponentTests/EmittedIL/ForLoop/ForLoop.fs
      • This consolidates the other two files by duplicating each test function and adding .RealInternalSignatureOff/.RealInternalSignatureOn suffixes to the generated baselines.
  • Updated:
    • tests/FSharp.Test.Utilities/ILChecker.fs
      • This is now unifying the assembly version for netstandard in the IL baselines, which lets us reuse more tests for both net472 and net8.0.

Checklist

  • This shouldn't need release notes.

Notes

These tests have always had some .opt-suffixed tests and some not, but verifyCompilation actually always calls withOptimize (that's why I didn't add opt/nonopt duplicates of the tests I added in #16650 to begin with). That means that some of the existing duplicate tests could be deduplicated, but I haven't done that in this PR.

* The test sources and baselines for these were duplicated in dotnet#15484 to
  test the realsig feature, but that meant that the test _sources_
  needed to be kept in sync by hand. This change uses the approach used
  in dotnet#16795 instead: it reuses the same test _source_ files and just
  adds duplicate _tests_ to check the emitted IL with realsig+.
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner May 9, 2024 14:26
Copy link
Contributor

github-actions bot commented May 9, 2024

✅ No release notes required

@KevinRansom KevinRansom enabled auto-merge (squash) May 13, 2024 17:34
@KevinRansom KevinRansom merged commit 7b0586c into dotnet:main May 13, 2024
32 checks passed
@brianrourkeboll brianrourkeboll deleted the forloop-realsig-consolidate branch May 13, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants