Fix e2e and integration tests #339
Conversation
2opremio
force-pushed
the
fix-system-tests
branch
from
d659679 to
66e4cbd
Compare
2opremio
force-pushed
the
fix-system-tests
branch
from
635dec5 to
848e41b
Compare
|
I think it’s fine that you merged this (since you were blocked) but the last commit (the default protocol stuff) was a bit of a hack. I am still waiting for @sreuland and @leighmcculloch to get to a conclusion on how to best solve this. The problem seems to stem from the system tests “stealling” QuickStart’s start script. |
| @@ -111,7 +114,8 @@ jobs: | |||
| build | |||
| - name: Run system test scenarios | |||
| run: | | |||
| docker run --rm -t --name e2e_test stellar/system-test:dev \ | |||
| docker run -e PROTOCOL_VERSION_DEFAULT=$PROTOCOL_VERSION_DEFAULT \ | |||
There was a problem hiding this comment.
this should be un-necessary to set at runtime, the image built in prior step should now have the proto version coded into it, and quickstart startup will just reference that
There was a problem hiding this comment.
The problem is that it's not really provided during runtime. I may be wrong, but can you point to were it's provided?
It is provided during build time, but AFAIU, it isn't saved anywhere.
There was a problem hiding this comment.
To clarify this even further. The quickstart image does this to make the default protocol permanent https://github.com/stellar/quickstart/blob/master/Dockerfile#L55-L57 but the system-test image AFAIU doesn't do that, it just borrows the start script from quickstart.
There was a problem hiding this comment.
systest image uses the qs image as the base/parent image, and invokes the qs start script from that base layer, so the image should have DEFAULT_PROTOCOL_VERSION hard coded as an ENV per latest on qs docker file
I need to do a PR that removes DEFAULT_PROTOCOL_VERSION from the run step only and see what the system test results are.
No description provided.