Write unit test for TaskView.get_title() #1083
Conversation
SqAtx
force-pushed
the
write_test_for_taskview
branch
from
f8017ed
to
f917b2c
Compare
nekohayo
added
the
maintainability
SqAtx
force-pushed
the
write_test_for_taskview
branch
3 times, most recently
from
dd7654a
to
a05a1a1
Compare
| text = self.task.content | ||
| title = self.task.title | ||
|
|
||
| # Insert text and tags as a non_undoable action, otherwise |
There was a problem hiding this comment.
This block of code has moved to the TaskView class. The logic hasn't changed besides the way the objects are named (self.insert() instead of self.textview.insert() etc) and the fact that the TextView calls is_new() on the task object.
| def test_get_title(self): | ||
| task_title = 'Very important task' | ||
| task = Task(id = uuid4(), title=task_title) | ||
| view = TaskView(Datastore(), task, None, False) |
There was a problem hiding this comment.
These 3 lines can probably be encapsulated into a create_task_view_from_task() test helper, but I can do that when I add another test.
|
Ugh okay, let's not merge that yet... GHA fails with "Fatal Python error: Segmentation fault", which is going to be a joy to debug. |
|
OK so I have confirmed that this is caused by |
SqAtx
force-pushed
the
write_test_for_taskview
branch
from
a05a1a1
to
328f6f1
Compare
SqAtx
force-pushed
the
write_test_for_taskview
branch
2 times, most recently
from
de7cb07
to
5cb9d90
Compare
|
I think this is worth merging even though I haven't solved the segfault issue (it anyone wants to look: https://github.com/getting-things-gnome/gtg/actions/runs/8864137269/job/24339006474?pr=1083), and even though it looks like the bug I set out to fix will be fixed by #1102 I'll work on unskipping the test separately. |
|
I managed to reproduce the segfault outside of the GitHub Action using a small server VM. The main problem seems to be that GA runs without any graphical display features, and the new GUI test wants to use these features. If you provide a fake display server, everything works fine: |
SqAtx
force-pushed
the
write_test_for_taskview
branch
from
5cb9d90
to
807466c
Compare
|
Thanks a lot for looking into it! That would have taken me a while to figure out. It feels a bit like a hack somehow? But I can't really articulate why, nor do I have a better way to fix the problem =D |
I think we can defend the move:
|
I started to look at getting-things-gnome#1077 and I thought it would be nice to be able to unit test this type of thing. A simple unit test for TaskView would be * create a task * create a view for it * check that the title of the view is the title of the task In the current state of affairs, this is difficult because the code that puts the Task data into the view lives in TaskEditor. But we pass a Task to the constructor of TaskView, so TaskView actually has all the data it needs. So I moved the logic there. I also moved the `is_new()` logic from the TaskEditor down to the Task. I feel like it should be possible to eventually do this in the TaskView constructor but for now, TaskEditor calls `self.textview.set_text_from_task()` from the same place it used to run that code. The test triggers a segmentation fault when running on GitHub Actions, so it's skipped there while I figure out why.
SqAtx
force-pushed
the
write_test_for_taskview
branch
from
807466c
to
3f833b4
Compare
|
Hi @SqAtx, this LGTM. Is this ready to merge or is there something else to do in this branch? |
|
@diegogangl It's ready to merge! It's a good first step. |
diegogangl
added this to the 0.7: "Getting things Gradius — Destroy the core!" milestone
|
Thanks! |
I started to look at #1077 and I thought it would be nice to be able to unit test this type of thing.
A simple unit test for TaskView would be
In the current state of affairs, this is difficult because the code that puts the Task data into the view lives in TaskEditor. But we pass a Task to the constructor of TaskView, so TaskView actually has all the data it needs. So I moved the logic there. I also moved the
is_new()logic from the TaskEditor down to the Task.I feel like it should be possible to eventually do this in the TaskView constructor but for now, TaskEditor calls
self.textview.set_text_from_task()from the same place it used to run that code.The test triggers a segmentation fault when running on GitHub Actions, so it's skipped there while I figure out why.