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

build: Run e2e tests against ic-ref #471

Merged
merged 9 commits into from
Apr 20, 2020
Merged

build: Run e2e tests against ic-ref #471

merged 9 commits into from
Apr 20, 2020

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Mar 20, 2020

this is a failed attempt; I don’t understand how this bats works or how to
debug it. @hansl, do you want to take over?

@nomeata nomeata changed the title e2e test: Run against ic-ref build: Run e2e tests against ic-ref Mar 20, 2020
@nomeata
Copy link
Contributor Author

nomeata commented Mar 20, 2020

Ok, this kinda works. I’ll hand this over to the SDK team, as I don’t know if they want this to be in master now and whether they want my horrible bash coding in master or want to clean it up.

@hansl hansl marked this pull request as ready for review March 22, 2020 02:13
@hansl hansl requested a review from a team March 22, 2020 02:13
@hansl hansl requested a review from a team as a code owner March 22, 2020 02:13
Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

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

LGTM, but will let someone else take a look since I had changes to it. I think @nomeata can also approve since his changes LGTM and I want approval for my changes ;)

@hansl
Copy link
Contributor

hansl commented Mar 22, 2020

I just did a shorthand for the USE_IC_REF stuff. It's more bash-y and easier to follow than strings.

@nomeata
Copy link
Contributor Author

nomeata commented Mar 22, 2020

I think @nomeata can also approve since his changes LGTM and I want approval for my changes ;)

Github won’t let me formally approve, as I created this PR. But your commits LGTM.

@eftychis
Copy link
Contributor

There is a connection error but the last changes don't seem to be the issue from a quick glance...

@hansl hansl mentioned this pull request Apr 20, 2020
5 tasks
@hansl hansl merged commit bac14b2 into master Apr 20, 2020
@hansl hansl deleted the joachim/e2e-ic-ref branch April 20, 2020 17:25
@nomeata
Copy link
Contributor Author

nomeata commented Apr 21, 2020

Looks the merge broke master. This could happen because it was no a strict merge, i.e. the branch was not updated before, so the merge commit created a situation on master that was never CI-checked.

A handfull of git bisect runs show that merging this PR breaks master from this commit on:
89e7669

(yay for git bisect + a well-stocked nix cache to fetch any revisions dfx)

This is not surprising: This PR pins a version of ic-ref-test that tries to be compatible with a prior, random version of replica to a new random version. By upgrading replica (and updating the code to match the changed behavior of replica) these tests break.

I’ll see what the precise differences are and if I can make a ic-ref tag that pretends to behave like that version for now. Else we need to revert this.

@nomeata
Copy link
Contributor Author

nomeata commented Apr 21, 2020

fix coming

nomeata added a commit that referenced this pull request Apr 21, 2020
in #471 we added running the e2e tests aginst the `legacy2` branch of
`ic-ref`, which was tailured to SDK’s behaviour at that time. That PR
was green.

But it was merged without updating the branch, and in the meantime SKD’s
`master` updated `replica` to 0.5.4, and also changed `dfx` accordingly.
At that version, `dfx` could no longer talk to `ic-ref`, and the test
made `master` red.

So I creaed a new branch `legacy3`, off `release-0.2` in `ic-ref`,
making the necessary adjustments to be compatible with `dfx` again.
These were:

 * Do not require CBOR tag
 * Make `sender` field optional
 * Do not check signatures
 * Do not require canister registration before installation
nomeata added a commit that referenced this pull request Apr 21, 2020
in #471 we added running the e2e tests aginst the `legacy2` branch of
`ic-ref`, which was tailured to SDK’s behaviour at that time. That PR
was green.

But it was merged without updating the branch, and in the meantime SKD’s
`master` updated `replica` to 0.5.4, and also changed `dfx` accordingly.
At that version, `dfx` could no longer talk to `ic-ref`, and the test
made `master` red.

So I creaed a new branch `legacy3`, off `release-0.2` in `ic-ref`,
making the necessary adjustments to be compatible with `dfx` again.
These were:

 * Do not require CBOR tag
 * Make `sender` field optional
 * Do not check signatures
 * Do not require canister registration before installation
dfinity-bot added a commit that referenced this pull request Nov 2, 2020
## Changelog for advisory-db:
Branch: master
Commits: [rustsec/advisory-db@a0e59ff2...65b9aa70](rustsec/advisory-db@a0e59ff...65b9aa7)

* [`0da539a2`](rustsec/advisory-db@0da539a) Add unmaintained crate advisory for `safe-nd` ([RustSec/advisory-db⁠#467](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/467))
* [`51fd5e3c`](rustsec/advisory-db@51fd5e3) Assigned RUSTSEC-2020-0063 to safe-nd ([RustSec/advisory-db⁠#469](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/469))
* [`3adba0fc`](rustsec/advisory-db@3adba0f) Add unmaintained crate advisory for `ffi_utils` ([RustSec/advisory-db⁠#464](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/464))
* [`74c2e86f`](rustsec/advisory-db@74c2e86) Assigned RUSTSEC-2020-0064 to ffi_utils ([RustSec/advisory-db⁠#470](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/470))
* [`a949bd46`](rustsec/advisory-db@a949bd4) Add unmaintained crate advisory for `fake_clock` ([RustSec/advisory-db⁠#465](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/465))
* [`00a4c19a`](rustsec/advisory-db@00a4c19) Assigned RUSTSEC-2020-0065 to fake_clock ([RustSec/advisory-db⁠#471](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/471))
* [`3761ab58`](rustsec/advisory-db@3761ab5) Add unmaintained crate advisory for `safe_bindgen` ([RustSec/advisory-db⁠#466](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/466))
* [`d5cf9d76`](rustsec/advisory-db@d5cf9d7) Assigned RUSTSEC-2020-0066 to safe_bindgen ([RustSec/advisory-db⁠#472](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/472))
* [`9757ff20`](rustsec/advisory-db@9757ff2) Add unmaintained crate advisory for `quic-p2p` ([RustSec/advisory-db⁠#468](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/468))
* [`65b9aa70`](rustsec/advisory-db@65b9aa7) Assigned RUSTSEC-2020-0067 to quic-p2p ([RustSec/advisory-db⁠#473](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/473))
mergify bot pushed a commit that referenced this pull request Nov 2, 2020
## Changelog for advisory-db:
Branch: master
Commits: [rustsec/advisory-db@a0e59ff2...65b9aa70](rustsec/advisory-db@a0e59ff...65b9aa7)

* [`0da539a2`](rustsec/advisory-db@0da539a) Add unmaintained crate advisory for `safe-nd` ([RustSec/advisory-db⁠#467](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/467))
* [`51fd5e3c`](rustsec/advisory-db@51fd5e3) Assigned RUSTSEC-2020-0063 to safe-nd ([RustSec/advisory-db⁠#469](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/469))
* [`3adba0fc`](rustsec/advisory-db@3adba0f) Add unmaintained crate advisory for `ffi_utils` ([RustSec/advisory-db⁠#464](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/464))
* [`74c2e86f`](rustsec/advisory-db@74c2e86) Assigned RUSTSEC-2020-0064 to ffi_utils ([RustSec/advisory-db⁠#470](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/470))
* [`a949bd46`](rustsec/advisory-db@a949bd4) Add unmaintained crate advisory for `fake_clock` ([RustSec/advisory-db⁠#465](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/465))
* [`00a4c19a`](rustsec/advisory-db@00a4c19) Assigned RUSTSEC-2020-0065 to fake_clock ([RustSec/advisory-db⁠#471](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/471))
* [`3761ab58`](rustsec/advisory-db@3761ab5) Add unmaintained crate advisory for `safe_bindgen` ([RustSec/advisory-db⁠#466](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/466))
* [`d5cf9d76`](rustsec/advisory-db@d5cf9d7) Assigned RUSTSEC-2020-0066 to safe_bindgen ([RustSec/advisory-db⁠#472](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/472))
* [`9757ff20`](rustsec/advisory-db@9757ff2) Add unmaintained crate advisory for `quic-p2p` ([RustSec/advisory-db⁠#468](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/468))
* [`65b9aa70`](rustsec/advisory-db@65b9aa7) Assigned RUSTSEC-2020-0067 to quic-p2p ([RustSec/advisory-db⁠#473](http://r.duckduckgo.com/l/?uddg=https://github.com/RustSec/advisory-db/issues/473))
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.

None yet

4 participants