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

fix: e2e tests crashing #1254

Merged
merged 6 commits into from
Aug 8, 2024
Merged

fix: e2e tests crashing #1254

merged 6 commits into from
Aug 8, 2024

Conversation

XavierChanth
Copy link
Member

@XavierChanth XavierChanth commented Aug 7, 2024

the dependency overrides should already be using ^0.6.0 as the minimum version since it is a transitive dependency of our override of dartssh2... but that would be asking too much of the toolchain.

- What I did

  • Hopefully this fixes the e2e tests

- How I did it

- How to verify it

- Description for the changelog
fix: e2e tests crashing

the dependency overrides *should* already be using ^0.6.0 as the minimum
version since it is a transitive dependency of our override of
dartssh2... but that would be asking too much of the toolchain.
@XavierChanth
Copy link
Member Author

Tested locally and the Dart tests are passing now, the C tests are flaking which is causing a timeout long enough for GNU parallel to exit with a timeout.

2024-08-07 15:10:50 | ERROR:     Failed after 2 attempts
2024-08-07 15:10:52 | ERROR:     Test FAILED : exit code 1
###
###
### NoPorts e2e test run complete at 2024-08-07 15:10:52
###
### Of a possible 180, 126 were not applicable (usually version constraints)
### Executed: 54  Passed: 48  Failed: 6

@XavierChanth XavierChanth changed the title fix: override pineacl version fix: e2e tests crashing Aug 7, 2024
@@ -42,12 +42,15 @@ if [ $allowParallelization == "true" ] && command -v env_parallel >/dev/null 2>&
export -f run_tests_for_daemon
# Run a round of tests against each daemon in parallel
parallel --jobs 5 \
--timeout 3m \
Copy link
Member Author

Choose a reason for hiding this comment

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

This ends up being 3minutes per daemon - which may be a bit short, we will have to see.

@@ -30,6 +30,7 @@ dependency_overrides:
git:
url: https://github.com/atsign-foundation/dartssh2
ref: trunk
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to make a transitive dependency explicit like this? My take is that we just need to bump the lock file(s)

@cpswan
Copy link
Member

cpswan commented Aug 8, 2024

After some discussion with @gkc I've backed out the changes to pubspec.yaml and introduced a pubspec.lock change that comes from dart pub upgrade dartssh2.

We should probably evaluate a mechanism to ensure that pubspec.lock is more frequently upgraded as there are a bunch of other outstanding bumps, which include some promotions from v0.x.x to v1.x.x.

@cpswan
Copy link
Member

cpswan commented Aug 8, 2024

@XavierChanth please merge if you're happy with the revised approach (or we can discuss in arch).

@XavierChanth XavierChanth merged commit e3820a0 into trunk Aug 8, 2024
6 checks passed
@XavierChanth
Copy link
Member Author

FYI @gkc @cpswan @JeremyTubongbanua , beyond these changes, the e2e tests were failing because I found some non-utf8 characters in the .ssh/authorized_keys file on the test run - The Dart daemon calls authKeys.readAsString() (authKeys is aFile) which does a utf8 decoding - this decode fails due to the non-utf8 characters. Not sure how this happened, but something worth noting in case we run into a similar issue.

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.

4 participants