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

Adding tests for origDatablocks #640

Merged
merged 13 commits into from
Aug 10, 2023
Merged

Adding tests for origDatablocks #640

merged 13 commits into from
Aug 10, 2023

Conversation

nitrosx
Copy link
Member

@nitrosx nitrosx commented Jul 27, 2023

Description

Added explicit testing for endpoint origdatablocks

Motivation

In the latest code review following issues 638, we realized that there was not any explicit testing for the origdatablocks endpoint.

Changes:

  • added file OrigDatablockForRawDataset.js

Tests included/Docs Updated?

  • Included for each change/fix?
  • Passing? (Merge will not be approved unless this is checked)
  • Docs updated?
  • New packages used/requires npm install?
  • Toggle added for new features?

Copy link
Collaborator

@martin-trajanovski martin-trajanovski left a comment

Choose a reason for hiding this comment

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

Looks good to me in general. The only thing is that this PR doesn't add only the tests for origDatablocks as it is saying in the title but also fixes some bugs I guess. Another thing is with the numbering of the tests. Not really sure if we need that or not.

@nitrosx
Copy link
Member Author

nitrosx commented Aug 10, 2023

Originally, this PR was meant to add only the tests, than I found issues with some of the endpoint (related to the tests that I added), so I addressed them here.
Regarding the numbering of the tests, they make it easier to find them in code... at least for me.

@nitrosx nitrosx merged commit e5006fa into master Aug 10, 2023
@nitrosx nitrosx deleted the github-issue-638 branch August 10, 2023 11:16
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.

2 participants