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

API V2: test that command is actually saved #9827

Merged
merged 2 commits into from
Dec 22, 2022
Merged

API V2: test that command is actually saved #9827

merged 2 commits into from
Dec 22, 2022

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Dec 20, 2022

I bug was introduced and our test didn't catch it because we weren't checking if the command was being saved.

@stsewd stsewd requested a review from a team as a code owner December 20, 2022 17:32
@stsewd stsewd requested a review from benjaoming December 20, 2022 17:32
self.assertEqual(build["commands"][0]["description"], "foo")
self.assertEqual(build["commands"][0]["exit_code"], 0)
self.assertEqual(
dateutil.parser.parse(build["commands"][0]["start_time"]), start_time
Copy link
Member Author

Choose a reason for hiding this comment

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

so annoying that drf doesn't return the date in python's isoformat compatible format :x

humitos added a commit that referenced this pull request Dec 21, 2022
Continuation of the work done in
#9815 that didn't work as we
expected.

We weren't saving the commands properly when hitting the API since
`serializers.SerializerMethodField` is a read-only field. This commit fixes this
problem by using a different Serializer depending if it's a GET request or not,
and if it's made by an Admin or not.

It overrides the method `BuildViewSet.get_serializer_class` to manage these
different cases and return the proper serializer.

Done on top of #9827 to be sure the
tests for saving commands are passing.
)

* Build details page: normalize/trim command paths (second attempt)

Continuation of the work done in
#9815 that didn't work as we
expected.

We weren't saving the commands properly when hitting the API since
`serializers.SerializerMethodField` is a read-only field. This commit fixes this
problem by using a different Serializer depending if it's a GET request or not,
and if it's made by an Admin or not.

It overrides the method `BuildViewSet.get_serializer_class` to manage these
different cases and return the proper serializer.

Done on top of #9827 to be sure the
tests for saving commands are passing.

* Feedback from review addressed

- rename serializer to mention "ReadOnly" on it's class name
- add docstrings to these serializers

* Typo when accessing the action

* Optional builder hash on local instance

Old builds don't have the builder hash in the docroot, since that was implmented
recently.

* Lint

* Apply suggestions from code review

Co-authored-by: Benjamin Balder Bach <[email protected]>

Co-authored-by: Benjamin Balder Bach <[email protected]>
@stsewd stsewd merged commit 0aa6e2c into main Dec 22, 2022
@stsewd stsewd deleted the test-command-save branch December 22, 2022 17:47
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.

2 participants