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

Add navvis files for unit tests #69

Closed
wants to merge 1 commit into from

Conversation

vjlux
Copy link
Collaborator

@vjlux vjlux commented Jun 19, 2024

Add NavVis test files for testing.

test data restored from https://github.com/microsoft/scantools/tree/main/test_data

Copy link
Contributor

Choose a reason for hiding this comment

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

Are these files needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

It has 0 bytes.

Unable to render rich display
Invalid image source.

Maybe it was intended.

@sarlinpe
Copy link
Collaborator

sarlinpe commented Jun 20, 2024

How large is this test data? I would very much avoid committing it to the repo, which would force all users to download it even though it is used only for testing. Instead, 1) add it as a git submodule or 2) zip it, add it as release artefact, and download it when running the tests. I prefer option 2.

@vjlux
Copy link
Collaborator Author

vjlux commented Jun 20, 2024

How large is this test data? I would very much avoid committing it to the repo, which would force all users to download it even though it is used only for testing. Instead, 1) add it as a git submodule or 2) zip it, add it as release artefact, and download it when running the tests. I prefer option 2.

yes we can do option 2 and add a comment if you want test

@vjlux vjlux closed this Jun 20, 2024
@vjlux vjlux deleted the user/lugruber/add_navvis_test_data branch June 20, 2024 12:29
@pablospe
Copy link
Contributor

I would very much avoid committing it to the repo, which would force all users to download it even though it is used only for testing

I agree, I was also proposing the same here :)

Now my question is, will we anyways download all this data when we do git clone (actually twice because it was in the other PR too). Is there a way to avoid this? I think @mihaidusmanu had this problem in the past. What did you do? Other than --depth 1 in main branch? Just curious, probably we would need to force push and rewrite the git history. Just raising the concern that not merging these commits is not enough to avoid downloading the data.

@sarlinpe
Copy link
Collaborator

sarlinpe commented Jun 24, 2024

By default git clone only clones the main branch, so it should not include any of these files - especially since the branch of this PR has now been deleted.

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.

3 participants