Skip to content

fix(test): Values check test should account for IgnoreInTests column option#431

Merged
kodiakhq[bot] merged 6 commits intomainfrom
fix/test/ignored-column
Nov 25, 2022
Merged

fix(test): Values check test should account for IgnoreInTests column option#431
kodiakhq[bot] merged 6 commits intomainfrom
fix/test/ignored-column

Conversation

@candiduslynx
Copy link
Copy Markdown
Contributor

No description provided.

@candiduslynx candiduslynx changed the title chore(test): IgnoreInTests column option should be accounted for values check test fix(test): IgnoreInTests column option should be accounted for values check test Nov 24, 2022
@candiduslynx candiduslynx changed the title fix(test): IgnoreInTests column option should be accounted for values check test fix(test): Values check test should account for IgnoreInTests column option Nov 24, 2022
@github-actions github-actions bot added fix and removed fix labels Nov 24, 2022
Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

Given it affects all plugin testing. we should have a test for that :)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 24, 2022

⏱️ Benchmark results

Comparing with 9882458

  • DefaultConcurrency-2 resources/s: 11,816 ⬇️ 2.35% decrease vs. 9882458
  • Glob-2 ns/op: 203.6 ⬆️ 22.50% increase vs. 9882458
  • TablesWithChildrenDefaultConcurrency-2 resources/s: 29,681 ⬆️ 1.17% increase vs. 9882458
  • BufferedScanner-2 ns/op: 12.99 ⬆️ 22.94% increase vs. 9882458
  • LogReader-2 ns/op: 37.87 ⬆️ 18.83% increase vs. 9882458

Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

almost there.

Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

Few issues:

  • no need to use testify package
  • no need to change function signature. just call t.Errorf directly instead of passing errors around.
  • use simple if else instead of switch

@candiduslynx candiduslynx force-pushed the fix/test/ignored-column branch from bd80026 to 5ef5e7a Compare November 25, 2022 10:33
@candiduslynx candiduslynx self-assigned this Nov 25, 2022
@kodiakhq kodiakhq bot merged commit ffffcd5 into main Nov 25, 2022
@kodiakhq kodiakhq bot deleted the fix/test/ignored-column branch November 25, 2022 10:39
candiduslynx pushed a commit that referenced this pull request Nov 25, 2022
🤖 I have created a release *beep* *boop*
---


##
[1.8.2](v1.8.1...v1.8.2)
(2022-11-25)


### Bug Fixes

* **test:** Values check test should account for `IgnoreInTests` column
option ([#431](#431))
([ffffcd5](ffffcd5))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants