Skip to content

Fix error in unmarked test script#2061

Merged
dpanici merged 2 commits intomasterfrom
dp/fix-test-script
Jan 16, 2026
Merged

Fix error in unmarked test script#2061
dpanici merged 2 commits intomasterfrom
dp/fix-test-script

Conversation

@dpanici
Copy link
Collaborator

@dpanici dpanici commented Jan 14, 2026

the head -n -2 was incorrect (-2 should just be 2), the script was silently failing locally for me (on macOS zsh) and causing it to not actually detect any unmarked tests even if they were present.

@dpanici dpanici requested review from a team, YigitElma, ddudt, f0uriest, rahulgaur104 and unalmis and removed request for a team January 14, 2026 17:04
@dpanici dpanici added the easy Short and simple to code or review label Jan 14, 2026
@dpanici dpanici changed the title Fix error in umarked test script Fix error in unmarked test script Jan 14, 2026
@f0uriest
Copy link
Member

Are there any tests we forgot to mark that weren't caught?

@YigitElma
Copy link
Collaborator

Are there any tests we forgot to mark that weren't caught?

I ran bash devtools/check_unmarked_tests.sh tests/ locally and couldn't find any missing tests.

@YigitElma
Copy link
Collaborator

On my system, both versions of the script can catch the missing tests (if I delete some existing ones), this might be OS dependent maybe?

@dpanici
Copy link
Collaborator Author

dpanici commented Jan 15, 2026

Are there any tests we forgot to mark that weren't caught?

Not that I saw

On my system, both versions of the script can catch the missing tests (if I delete some existing ones), this might be OS dependent maybe?

Hm I ran on macOS. But if the version in this PR works for both linux and macOS I'd prefer this version

@dpanici dpanici merged commit 5a3da85 into master Jan 16, 2026
25 checks passed
@dpanici dpanici deleted the dp/fix-test-script branch January 16, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

easy Short and simple to code or review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments