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

Update testing README #2855

Merged
merged 5 commits into from
Sep 21, 2024
Merged

Update testing README #2855

merged 5 commits into from
Sep 21, 2024

Conversation

daljit46
Copy link
Member

@daljit46 daljit46 commented Mar 8, 2024

Addresses #2826.
Testing tips and instructions have been updated to reflect the transition to CMake. Sections on the handling of testing data assume #2848 has been merged.
Some instructions may not be clear or need removing due to the fact that some of the concerns/workflows no longer apply with the transition to CMake.

It's still WIP.

@daljit46 daljit46 requested a review from Lestropie March 8, 2024 14:03
@daljit46 daljit46 self-assigned this Mar 8, 2024
@daljit46 daljit46 marked this pull request as draft March 8, 2024 14:03
Copy link

github-actions bot commented Mar 8, 2024

clang-tidy review says "All clean, LGTM! 👍"

@daljit46 daljit46 force-pushed the update_testing_readme branch from edf9a4e to 5d165f8 Compare March 14, 2024 16:26
Copy link

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@daljit46 daljit46 mentioned this pull request May 7, 2024
9 tasks
@daljit46 daljit46 force-pushed the update_testing_readme branch from cb5f167 to 209526c Compare September 12, 2024 12:46
@daljit46
Copy link
Member Author

I've updated the testing instructions to reflect the latest changes in testing code. @Lestropie could you please have a look and see if there's anything that's missing and/or incorrect?

Copy link

clang-tidy review says "All clean, LGTM! 👍"

@daljit46 daljit46 force-pushed the update_testing_readme branch from 209526c to b4d638b Compare September 12, 2024 12:48
Copy link

clang-tidy review says "All clean, LGTM! 👍"

Copy link

clang-tidy review says "All clean, LGTM! 👍"

@Lestropie
Copy link
Member

  • There are now labels both for language and for each command name, so no need to use regexes.
  • There were still some out-of-date references to being one test per line of a test script file, predating Testing: shell script per test #2865.
  • The later text read like it was written when we first added the testing. I've replaced it with something new describing how I typically go about it these days.

@daljit46
Copy link
Member Author

I think you've removed the section on using a local clone to download the test data. Shall we omit that in the testing readme? I think it'd be for that mechanism to be documented somewhere at least.

@Lestropie
Copy link
Member

Good catch. Think I failed to move it partway through, then erroneously deleted it entirely when I had to do an explicit rebase.

@Lestropie Lestropie marked this pull request as ready for review September 13, 2024 12:02
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@daljit46
Copy link
Member Author

This seems to be good to go IMO, so will merge soon.

@Lestropie Lestropie merged commit ca13e7f into dev Sep 21, 2024
6 checks passed
@Lestropie Lestropie deleted the update_testing_readme branch September 21, 2024 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants