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

Unit Tests Need Updating #67

Closed
butlerpd opened this issue Apr 21, 2024 · 1 comment
Closed

Unit Tests Need Updating #67

butlerpd opened this issue Apr 21, 2024 · 1 comment
Assignees
Labels
BLOCKING Required before a new release

Comments

@butlerpd
Copy link
Member

Merging the SasView PR that got rid of 1D data in the sasview/example_data (and possibly some others?) broke the tests relying on pulling data from that repo. This is causing all pull request in sasdata to fail and needs to be fixed asap.

Two questions however:

  • I do not understand the test of local file vs URL. Is this test no run only on build? If so what is "remote" in this case. Pulling from the right directory is a thing I suppose but I'm missing the point of this test I think?
  • Why are we pulling from example_data for the PDH data (or any other data) rather than the test\data folder like we are for most others. I note that the PDH data exists in both folders (both on sasdata not in sasview anymore). Again maybe I'm missing something here?
@butlerpd butlerpd added the BLOCKING Required before a new release label Apr 21, 2024
@krzywon
Copy link
Collaborator

krzywon commented Apr 23, 2024

I'm working on this now. Expect a PR shortly. To answer your questions:

  • The local vs. URL is to test the cloned repo on the build system as compared to fetching data from a 'remote' system. The runners are on Github, but the local data is cloned directly onto the runners servers.
  • The PDH data set was moved from sasview to sasdata, but was never in the unit test data in the master branch until after the original PR was merged. This was supposed to be ticketed and fixed soon after the original PR was merged. There is even a TODO statement in the code outlining this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLOCKING Required before a new release
Projects
None yet
Development

No branches or pull requests

2 participants