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

fix: Authors assignment not work when editing and creating new learning units #268

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

palfner-sse
Copy link
Contributor

@palfner-sse palfner-sse commented Feb 3, 2025

closes #267

@palfner-sse palfner-sse self-assigned this Feb 3, 2025
@spark-sse
Copy link
Member

kriegst du da noch ein easy test hin, ob der button clickable ist?

spark-sse
spark-sse previously approved these changes Feb 3, 2025
@palfner-sse
Copy link
Contributor Author

Tests sind jetzt da. Aber die Pipeline geht kaputt. Und zwar im feature-completion test. Ich weiß nicht, ob ich daran Schuld bin oder nicht. Im Master geht das auch Kaputt. Aber hier dann auch am Anfang nicht. Wenn dieser Git Error nicht mehr da ist, muss ich schauen, wo das herkommt. (Maby ist das ja auch nur ein Lokales Problem.)

@spark-sse
Copy link
Member

war ein jenkins fehler

Copy link
Member

@spark-sse spark-sse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"weil es nicht anders geht", war deine Antwort auf meine Frage, was ist der mehrwehrt der user-event-library. Das ist für mich ein Warnsignal.

ich hab es mir mal angeschaut und dir hat lediglich ein waitFor um das expect gefehlt. Das hättest du auch in den anderen Tests in unserer codebase nachschauen können

Bitte sei hier sorgfältiger. Der Idee der user-event-library ja nicht schlecht, aber wir sollten sie wenn dann aus den richtigen gründen einsetzen. Ich hab die lib wieder entfernt. Siehe letzter commit

@palfner-sse
Copy link
Contributor Author

palfner-sse commented Feb 14, 2025

"weil es nicht anders geht", war deine Antwort auf meine Frage, was ist der mehrwehrt der user-event-library. Das ist für mich ein Warnsignal.

Ich weiß nicht, wieso das so passiert. Aber wenn du waitFor nutzt, musst du auch "await" benutzen, weil die Function Async ist, sonst kann ich den Button nicht raussuchen. Denn wenn das nicht passiert, wird das expect(onCloseMock).toHaveBeenCalledWith(fakeAuthors[0]); gar nicht zu Ende gemacht, bis der Test zu Ende ist. Wenn man das await hinzufügt, sieht man das die Function nie Aufegrufen wird. Ich weiß nach wie vor nicht, warum fireEvent das nicht anklickt. Aber so kann es nicht bleibe, weil der Test halt jetzt immer grün wird. Das ist am leichtesten zu sehen wenn man dem Mock ein Console log hinzufügtjest.fn(() => {console.log("Mock function has been called");}); .

@palfner-sse
Copy link
Contributor Author

Was wird nun aus den Tests?

@spark-sse
Copy link
Member

Du hast recht, ich habe ein await vergessen was dort hinzugefügt werden muss. D.h. das eigentlich problem ist, dass fireEvent das nicht richtig auslöst. Vielleicht ein propagation problem? Auch wenn es mich ein wenig wurmt, nicht zu wissen was genau das problem, ist es jetzt den zeitaufwand nicht mehr wert.

@palfner-sse
Copy link
Contributor Author

Habe noch schnell die Namen von den Functionen angepasst, ich denke das ist jetzt Ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authors assignment not work when editing and creating new learning units
2 participants