Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

test: make refs tests iteration order independent #2982

Merged
merged 2 commits into from
Jun 8, 2020
Merged

test: make refs tests iteration order independent #2982

merged 2 commits into from
Jun 8, 2020

Conversation

koivunej
Copy link
Contributor

Proposing here a change which will make the refs.js tests dag node iteration order independent by sorting the expected and actual refs list. In the soon-to-be polished rust-ipfs refs implementation the refs are iterated breadth-first. Looking at the js-ipfs implementation I am not 100% sure but looks like the implementation is recursive depth-first (I could be wrong).

The iteration orders differ on almost half of the tests, but only big difference is with the test modified on the second commit: recursive+unique test with formatting <linkname> where a document has two names and breadth-first and depth-first offer the different names as a result.

The proposed changes are similar to what is already done for the dag-cbor nodes (sorted).

@koivunej
Copy link
Contributor Author

I think the travis failure is transient. This time I did run npm run lint for sure :)

@koivunej
Copy link
Contributor Author

@hugomrdias this is continuation to #2972 and Marks #2980 but this change, while small in lines changed is somewhat larger on another level, the iteration order. I couldn't find any specs for api/v0/refs that would had locked in any iteration order, so when implementing the rust-ipfs one I just went with the breadth-first one which is more familiar for me as I always try to stay clear of recursion.

@achingbrain
Copy link
Member

Master had a temporary GH url for one of the dependencies - the branch the url referenced has since been merged & deleted.

The GH url has since been removed from master so this PR just needs rebasing for CI to run.

sorting allows any iteration order.
cids instead of linknames doesn't lock down the iteration algorithm.
@koivunej
Copy link
Contributor Author

Test failures look unrelated but similar to last time.

@aphelionz
Copy link
Contributor

Hi @achingbrain just pinging on this - is the CI something on our end or is it just a flaky run?

@achingbrain achingbrain changed the title Make refs tests iteration order independent test: make refs tests iteration order independent Jun 8, 2020
@achingbrain achingbrain merged commit a8b4082 into ipfs:master Jun 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants