Add test for github format of doctest report#39512
Conversation
|
Documentation preview for this PR (built with commit 3703fb7; changes) is ready! 🎉 |
|
I am curious why the text "Failed example" shows twice (once in red and once in black), as seen: 
? Is this intended? |
|
It has always been like that, but looking at what the program prints out a reasonable guess is the part being put after On the other hand in the downloaded log file only would show up. This actually slightly inconvenience me in that I can't just download the log and parse it for the file and line number. (side note: There should probably be more tests for the |
Yes, it seems so. Thanks.
I agree. Maybe something to be improved later, if possible. |
|
this tests for whole-file timeout and failed example, although using unittest instead of command-line-based test. the long time warnings are tested in #40413, so this should be good. |
sagemathgh-39512: Add test for github format of doctest report As in the title. The feature was added in sagemath#36938 for the purpose of showing GitHub annotations for test failures, but there wasn't a doctest for it. This adds the test. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (no change to documentation preview because private method.) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39512 Reported by: user202729 Reviewer(s): Dima Pasechnik
sagemathgh-39512: Add test for github format of doctest report As in the title. The feature was added in sagemath#36938 for the purpose of showing GitHub annotations for test failures, but there wasn't a doctest for it. This adds the test. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (no change to documentation preview because private method.) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39512 Reported by: user202729 Reviewer(s): Dima Pasechnik
sagemathgh-39512: Add test for github format of doctest report As in the title. The feature was added in sagemath#36938 for the purpose of showing GitHub annotations for test failures, but there wasn't a doctest for it. This adds the test. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (no change to documentation preview because private method.) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39512 Reported by: user202729 Reviewer(s): Dima Pasechnik
|
Why is this PR still open? Anyway I was waiting for an answer to the question:
|
|
Ah? I can't remember what happened. Probably because the merge accidentally remove the positive-review and I didn't notice. Anyway, about line 1261, After re-reading, the immediate next doctest reset Also, this is the first time I see that comment. Maybe you didn't click "submit comments" button? |
OK.
I don't know. Perhaps...since I see "Pending" tags attached to the comments. |
|
@vbraun - something fell through with the merge here? |
|
more likely is I accidentally make a merge commit (93459a6) which sets the status back to need review, and I didn't notice it. |
|
then it's already merged? you can check if these changes are in the latest beta. |
|
no, as in I accidentally merge latest develop into this branch, which leads to the CI being rerun and synchronize labels CI workflow set positive-review to needs-review. (well known problem with the synchronize labels CI workflow.) |
sagemathgh-39512: Add test for github format of doctest report As in the title. The feature was added in sagemath#36938 for the purpose of showing GitHub annotations for test failures, but there wasn't a doctest for it. This adds the test. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (no change to documentation preview because private method.) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39512 Reported by: user202729 Reviewer(s): Dima Pasechnik, user202729
As in the title.
The feature was added in #36938 for the purpose of showing GitHub annotations for test failures, but there wasn't a doctest for it. This adds the test.
📝 Checklist
⌛ Dependencies