Skip to content

unit test writer package#304

Merged
general-kroll-4-life merged 5 commits into
stackql:mainfrom
tomekz:unit-test-writer
Oct 9, 2023
Merged

unit test writer package#304
general-kroll-4-life merged 5 commits into
stackql:mainfrom
tomekz:unit-test-writer

Conversation

@tomekz
Copy link
Copy Markdown
Contributor

@tomekz tomekz commented Oct 7, 2023

Description

Added unit tests for internal/writer package

Type of change

  • Bug fix (non-breaking change to fix a bug).
  • Feature (non-breaking change to add functionality).
  • Breaking change.
  • Other

Issues referenced.

#280

Evidence

Screenshot 2023-10-07 at 21 10 00

Checklist:

  • A full round of testing has been completed, and there are no test failures as a result of these changes.
  • The changes are covered with functional and/or integration robot testing.
  • The changes work on all supported platforms.

Variations

Tech Debt

@general-kroll-4-life
Copy link
Copy Markdown
Contributor

general-kroll-4-life commented Oct 7, 2023

Cheers for this. Running the checks now. Please do one favour and run go mod tidy and then commit and push any changes.

Thanks again

@tomekz
Copy link
Copy Markdown
Contributor Author

tomekz commented Oct 8, 2023

Cheers for this. Running the checks now. Please do one favour and run go mod tidy and then commit and push any changes.

Thanks again

Done

@tomekz
Copy link
Copy Markdown
Contributor Author

tomekz commented Oct 8, 2023

The "Windows Build" CI job is failing. I sadly can't tell which of the introduced changes caused that

@general-kroll-4-life
Copy link
Copy Markdown
Contributor

general-kroll-4-life commented Oct 8, 2023

The "Windows Build" CI job is failing. I sadly can't tell which of the introduced changes caused that

@tomekz yes I shall look into it and also it is past time to make the testing failure dumps more useful. Shall update when i can

@general-kroll-4-life
Copy link
Copy Markdown
Contributor

general-kroll-4-life commented Oct 9, 2023 •
edited
Loading

@tomekz ok finally got into this. It is some windows specific stuff we have done. Have got it working and verified as per this CI action. The changes are in commit 991c14a. You can either cherry-pick this commit and push again, or check the PR box to “Allow edits from maintainers”, or do something else I have not thought of; totally up to you 😎

@tomekz
Copy link
Copy Markdown
Contributor Author

tomekz commented Oct 9, 2023

Thanks a lot. I've applied the fix for my branch

@general-kroll-4-life
Copy link
Copy Markdown
Contributor

general-kroll-4-life commented Oct 9, 2023

@tomekz cool cheers. FYI, presuming this passes, I'm intending to squash and merge. Much appreciate the good work

@general-kroll-4-life
Copy link
Copy Markdown
Contributor

general-kroll-4-life commented Oct 9, 2023

@tomekz all checks are passed and it is approved. Please go ahead and squash and merge yourself... hopefully we have set things up so that is possible.

@tomekz
Copy link
Copy Markdown
Contributor Author

tomekz commented Oct 9, 2023

Thanks @general-kroll-4-life , I'm glad I could help. Can I ask you to merge it as I don't have write access to this repo and can't merge it myself

@general-kroll-4-life general-kroll-4-life merged commit e165895 into stackql:main Oct 9, 2023
@sanketmp sanketmp mentioned this pull request Oct 16, 2023
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@general-kroll-4-life general-kroll-4-life general-kroll-4-life approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tomekz @general-kroll-4-life