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

ci: set behavior test ci for aliyun drive #4657

Merged
merged 4 commits into from
Jun 18, 2024
Merged

ci: set behavior test ci for aliyun drive #4657

merged 4 commits into from
Jun 18, 2024

Conversation

suyanhanx
Copy link
Member

No description provided.

@Xuanwo
Copy link
Member

Xuanwo commented May 31, 2024

aliyun drive test failed for

failures:

---- behavior::test_list_empty_dir ----
Unexpected (persistent) at List::next, context: { service: aliyun_drive, path: / } => The resource file cannot be found. file not exist


failures:
    behavior::test_list_empty_dir

test result: FAILED. 113 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 291.46s

@suyanhanx
Copy link
Member Author

aliyun drive test failed for

Thanks. I'm handling this on my local machine.

@suyanhanx
Copy link
Member Author

The env var RUST_TEST_THREADS doesn't seem to be working properly.

@Xuanwo
Copy link
Member

Xuanwo commented Jun 4, 2024

I feel like the implemention has some bug to address. For example:

---- behavior::test_list_empty_dir ----
Unexpected (persistent) at List::next, context: { service: aliyun_drive, path: / } => The resource file cannot be found. file not exist

List empty dir should return None instead of not found?

@suyanhanx
Copy link
Member Author

It's superficial. Most of those errors reached the limit of read/write.

@yuchanns

This comment was marked as outdated.

@Xuanwo
Copy link
Member

Xuanwo commented Jun 17, 2024

As I mentioned, the tests go well when limited to 1 thread:

Odd, we've already set test_threads=1.

- name: Setup test threads
shell: bash
run: |
cat << EOF >> $GITHUB_ENV
RUST_TEST_THREADS=1
EOF

@yuchanns

This comment was marked as outdated.

@Xuanwo
Copy link
Member

Xuanwo commented Jun 17, 2024

Based on my testing on my host, RUST_TEST_THREADS=1 does not behave the same as --test-threads=1.

But docs said

This can also be specified with the RUST_TEST_THREADS environment variable.

https://doc.rust-lang.org/rustc/tests/index.html#--test-threads-num_threads

@suyanhanx
Copy link
Member Author

It's a bit strange. The env var is also behaving differently on my local machine.

I found this:
rust-lang/rust#104053

@suyanhanx
Copy link
Member Author

image image image

@yuchanns
Copy link
Member

yuchanns commented Jun 17, 2024

I rewrite the lister (without getting file details), and LGTM on green. Previously, the test-check would list items while other threads delete things, leading to the random 404 files not found.

@suyanhanx suyanhanx force-pushed the ci_aliyun_drive branch 2 times, most recently from b7e0fd8 to a1d15d9 Compare June 17, 2024 13:36
@Xuanwo
Copy link
Member

Xuanwo commented Jun 17, 2024

Wow, so cool! Is this PR ready to go?

@suyanhanx suyanhanx marked this pull request as ready for review June 17, 2024 14:08
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@Xuanwo Xuanwo merged commit 8d39e5b into main Jun 18, 2024
247 checks passed
@Xuanwo Xuanwo deleted the ci_aliyun_drive branch June 18, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants