gtristan commented on PR #1994:
URL: https://github.com/apache/buildstream/pull/1994#issuecomment-2716977702
Regarding testing in this PR: Yes the added test is good to add, but not at
all sufficient IMO.
What I would like to see here...
* Take the `tests/frontend/artifact_checkout.py` test as an example of a
test that:
* Actually builds something
* Pushing the artifact to a remote artifact cache
* Make sure you create a new directory for the test project data
* In the past we had many tests using the same directory, e.g.
`tests/frontend/project` still has a bit of this mess going on
* We made efforts to tear this apart ensuring that individual tests have
their own static committed project directories, please take this new approach
as it is easier to maintain, and modifying one test does not subtly affect
adjacent tests
* You can add this new test to `tests/frontend/show.py`, or to a new python
file (do not sneak the new unrelated test additional test into
`artifact_checkout.py` of course ;-) )
* The test, like the artifact checkout test, can use a simple `import`
element of a committed text file
* This is expected to be 100% reproducible, so the CAS digest should
always be the same
* Record this CAS digest (whether we call this a "hash" or whatever in the
final CLI UI, just take note of it)
* The test body essentially does this:
* Build the element
* Run `bst show --format ...` to obtain the CAS digest
* Assert that the CAS digest is _exactly the one you previously recorded_,
i.e. hard coded CAS digest
* Test multiple scenarios
* Building with and without the remote cache enabled
* Building with remote cache enabled, deleting the artifact locally,
before `bst show`
* In this scenario I think we expect to not have the CAS digest
* Building with remote cache enabled, delete the artifact locally, then
`bst artifact pull` the artifact, and `bst show`
* In this scenario we expect to have exactly the hard coded CAS digest
I realize this is a lot of text, but it shouldn't be too difficult to do in
practice, there are examples of all of this in the tests already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
