[ENG-10364] Part 1: Add unit tests for preprints#895
[ENG-10364] Part 1: Add unit tests for preprints#895brianjgeiger merged 5 commits intoCenterForOpenScience:feature/pbs-26-2from
Conversation
brianjgeiger
left a comment
brianjgeiger
left a comment
There was a problem hiding this comment.
A couple of questions, and one test that looks like it might not be testing everything it says it is.
src/app/features/preprints/pages/create-new-version/create-new-version.component.html
Show resolved
Hide resolved
| it('should compute latest withdrawal request correctly', () => { | ||
| const latestRequest = component.latestWithdrawalRequest(); | ||
| expect(latestRequest).toBe(mockWithdrawalRequests[0]); | ||
| it('should show toast error for 409 on create new version', () => { |
There was a problem hiding this comment.
Is this the ideal workflow here?
There was a problem hiding this comment.
What do you mean? I can simplify it.
There was a problem hiding this comment.
I mean more from a user perspective, should we ever actually 409, or should the front-end prevent that from happening?
There was a problem hiding this comment.
I just added tests to cover the existing code. I’m not sure when or under what conditions this should happen for the user. I assume there was a specific case for it, so it was added.
| it('should compute is pending withdrawal', () => { | ||
| const pending = component.isPendingWithdrawal(); | ||
| expect(typeof pending).toBe('boolean'); | ||
| it('should show edit button for latest or initial preprint', () => { |
There was a problem hiding this comment.
Is this actually also testing for latest version, or just initial version?
There was a problem hiding this comment.
It tests initial version. I will change name.
There was a problem hiding this comment.
Could we add another test to ensure that it shows edit button for latest preprints? I think I saw a bug report about that coming to us soon.
brianjgeiger
left a comment
brianjgeiger
left a comment
There was a problem hiding this comment.
A couple of questions, and one test that looks like it might not be testing everything it says it is.
brianjgeiger
left a comment
brianjgeiger
left a comment
There was a problem hiding this comment.
Add one test and I think we're good.
brianjgeiger
left a comment
brianjgeiger
left a comment
There was a problem hiding this comment.
This is good once tests run.
brianjgeiger
merged commit 2ae4c46
into
CenterForOpenScience:feature/pbs-26-2
2 participants
Summary of Changes