Skip to content

Conversation

@Urhengulas
Copy link
Contributor

@Urhengulas Urhengulas commented Jan 4, 2024

This PR switches from assert_debug_eq to assert_eq and only compares parts of the result and not the whole. The aim is to only compare parts which are relevant to the test and also make it more readable.

Part of #14268.

Questions

  • Can I use Vec? If not, what is the alternative?
    I assume I cannot because of:
    **Architecture Invariant:** rust-analyzer tests do not use libcore or libstd.
  • Should I group it by file, as proposed by Lukas?
    file_id 1:
        source_file_edits: 
            - Indel { insert: "foo2", delete: 4..7 }
    
    file_id 2:
        file_system_edits:
            MoveFile AnchoredPathBuf { anchor: FileId(2), path: "foo2.rs", }
    
  • Is it okay to ignore CreateFile events? They do not have a FileId, which would be problematic, but they do not occur in the existing tests, so I marked them as unreachable!() so far.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 4, 2024
@Veykril
Copy link
Member

Veykril commented Jan 4, 2024

Can I use Vec? If not, what is the alternative?

This is meant for test fixtures (the code that is passed to tests as strings), those dont see std library stuff. Testing infra is free to use whatever.

Should I group it by file, as proposed by Lukas?

On second though thats probably not worth it given the number of files changed is usually 1 anyways

@Urhengulas Urhengulas marked this pull request as ready for review January 4, 2024 13:30
@Veykril
Copy link
Member

Veykril commented Jan 4, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jan 4, 2024

📌 Commit 656ac10 has been approved by Veykril

It is now in the queue for this repository.

@Veykril Veykril changed the title Switch to expected.assert_eq for ide tests internal: Switch to expected.assert_eq for ide tests Jan 4, 2024
@bors
Copy link
Contributor

bors commented Jan 4, 2024

⌛ Testing commit 656ac10 with merge 7f75815...

@bors
Copy link
Contributor

bors commented Jan 4, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 7f75815 to master...

@bors bors merged commit 7f75815 into rust-lang:master Jan 4, 2024
@Urhengulas Urhengulas deleted the dont-assert-debug branch January 4, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants