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

Tests to remove old stage1 labeled issues #21365

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

RetroDev256
Copy link
Contributor

@RetroDev256 RetroDev256 commented Sep 9, 2024

My methodology:
attempt to recreate & minimize the original bug as much as possible, then change only what is necessary to convert the syntax directly to the latest version of zig. By this, any compiler error resulting from form or syntax (if it still existed, which it 99% does not) would be tested.

Closes #7724
Closes #7816
Closes #7841
Closes #8146
Closes #7370
Closes #7789
Closes #7865
Closes #8788
Closes #8501
Closes #8401
Closes #8547
Closes #10425
Closes #10284
Closes #6624
Closes #8902
Closes #9972 (same as #8902)
Closes #9887 (same as #8902)
Closes #8986
Closes #9021
Closes #9085
Closes #9434
Closes #9469
Closes #8640
Closes #9284
Closes #9600
Closes #8320
Closes #10024
Closes #9401 (same as #10024)

@RetroDev256
Copy link
Contributor Author

RetroDev256 commented Feb 5, 2025

Running it locally I get Error.invalidIncrementalTestIndex on merge_error_sets.2.zig:

/// Iterates a set of filenames extracting batches that are either incremental
/// ("foo.0.zig", "foo.1.zig", etc.) or independent ("foo.zig", "bar.zig", etc.).
/// Assumes filenames are sorted.
const TestIterator = struct {
    start: usize = 0,
    end: usize = 0,
    filenames: []const []const u8,
    /// reset on each call to `next`
    index: usize = 0,

    const Error = error{InvalidIncrementalTestIndex};
    ...

This test is independent, so I should probably change this filename.
When I made this PR, I didn't quite understand how the filenames worked - do the others look fine?

@alexrp
Copy link
Member

alexrp commented Feb 5, 2025

@mlugg can you weigh in on the above?

@RetroDev256
Copy link
Contributor Author

RetroDev256 commented Feb 5, 2025

Until we get an answer, I'll just rename the offending case :)

I also noticed that the wasm32-wasi target isn't correctly compiling most of the test cases:
image

I'll limit the offending tests to // target=x86_64-linux,x86_64-macos,x86_64-windows for now, unless something else would be preferred.

@mlugg
Copy link
Member

mlugg commented Feb 5, 2025

Hah, sorry about that. These .0.zig etc files are our old incremental test harness. I meant to rip that functionality out, but apparently forgot. I'll see if I can do that in a bit.

That said, I suggest renaming anyway (_0.zig or something) to avoid confusion with the old system.

At a glance, I have a few more review comments here:

  • Almost every test which isn't in compile_errors/ should be a behavior test, not a test case. Separated cases which aren't for compile errors or safety checks are exceedingly rare.
    • The only exception I can see at a glance is access_field_structs_via_assembly (did you mean "struct fields" rather than "field structs"?), because it's tied to one arch due to inline asm so IMO makes more sense as a case.
  • Never specify backend=stage2; it's the default.
  • Don't specify explicit target= unless you need to. The default is target=native, which is a good default. Your cases limit them to x86_64 on a few targets for no good reason. Unless you have a specific reason to restrict a case to an exact target -- e.g. it depends on a specific calling convention or inline asm syntax -- don't!

…ood news, the bug reproduces on 0.7.1 with a packed struct as well, so it is likely the same bug.
@RetroDev256
Copy link
Contributor Author

RetroDev256 commented Feb 5, 2025

What exactly are these failed tests? They look like platform specific tests, but I don't know what part of the PR is causing them:

Build Summary: 7556/7909 steps succeeded; 344 skipped; 3 failed; 8564/8793 tests passed; 229 skipped
test transitive failure
+- test-modules transitive failure
   +- test-behavior transitive failure
      +- run test behavior-x86_64-windows.win11_ge...win11_ge-gnu-skylake-Debug transitive failure
      |  +- zig test Debug native failure
      +- run test behavior-x86_64-windows.win11_ge...win11_ge-gnu-skylake-Debug-libc transitive failure
      |  +- zig test Debug native failure
      +- run test behavior-x86_64-windows.win11_ge...win11_ge-gnu-skylake-Debug-single transitive failure
         +- zig test Debug native failure

Ok, it might be a intern pool or linker bug?

error(link): NAV behavior.export_keyword.test.exported zero length concatenated arrays__struct_78171.concatenated_empty_arrays(InternPool.Nav.Index(22961)) assigned symbol 69822 but not allocated!

Update: I have determined that the failure happens when builtin.zig_backend == .stage2_x86_64. Weird stuff happens when you export a [0]u8, I guess.

Update: While I don't know which remaining tests are failing, I suspect they are a combination of wasm backend tests and c backend tests. Turns out these old bug reports can indeed be useful! I'll see if I can get a windows virtual machine up and running so I can continue this endeavor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment