Skip to content

Double-check cfg_version test coverage #141452

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

Open
jieyouxu opened this issue May 23, 2025 · 1 comment
Open

Double-check cfg_version test coverage #141452

jieyouxu opened this issue May 23, 2025 · 1 comment
Assignees
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-cfg_version `#![feature(cfg_version)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

          Discussion: this test is kinda testing multiple things and bears multiple responsibilities, we may want to split this test up:
  1. We wish to exercise the feature-gating of the unstable feature itself. That is, I would expect #[cfg(version("1.49.0"))] to be this feature-gate test without #![feature(cfg_version)] and the user should get the "this is unstable" error message.
  2. We wish to exercise the wellformed-ness of the values passed to version(..) as a sort of "interface". In particular, I'd consider splitting tests for:
    1. The syntactical wellformedness:
      • Proper name-with-paren-value form (version(..)): known vs unknown major version string "1", known vs unknown major-minor version string, known vs unknown major-minor-patch version string, empty version string, some "hello world" nonsense version string.
      • Invalid forms (version = ".." or bare version).
    2. The semantic wellformedness of the version string: major version (e.g. "1"), major-minor version (e.g. "1.1"), major-minor-patch (e.g. "1.1.1"), and also known-vs-unknown variants for them.
  3. I'd also check that if user manually specifies --cfg version or --cfg version=".." they are not affected and usable with or without feature gate (I need to double-check that).

This is also fine as a follow-up, I can probably write those coverage.

Originally posted by @jieyouxu in #141413 (comment)

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 23, 2025
@jieyouxu jieyouxu self-assigned this May 23, 2025
@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-cfg_version `#![feature(cfg_version)]` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 23, 2025
@est31
Copy link
Member

est31 commented May 24, 2025

As written in #141413 (comment), test feature-gate-cfg-version.rs covers most of the points (1, 2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-cfg_version `#![feature(cfg_version)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants