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

fix(i): Flaky normalize filter test #1912

Merged
merged 3 commits into from
Sep 27, 2023

Conversation

nasdf
Copy link
Member

@nasdf nasdf commented Sep 27, 2023

Relevant issue(s)

Resolves #1879

Description

This PR attempts to fix a flaky filter test. The normalization logic has been refactored to a recursive / functional approach that (hopefully) makes it a little easier to understand.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Ran normalize tests in a loop for 200k+ iterations.

Specify the platform(s) on which this was tested:

  • MacOS

@nasdf nasdf added bug Something isn't working area/planner Related to the planner system labels Sep 27, 2023
@nasdf nasdf added this to the DefraDB v0.8 milestone Sep 27, 2023
@nasdf nasdf self-assigned this Sep 27, 2023
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (db1f41c) 70.76% compared to head (21498d1) 70.75%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1912      +/-   ##
===========================================
- Coverage    70.76%   70.75%   -0.01%     
===========================================
  Files          233      233              
  Lines        24295    24322      +27     
===========================================
+ Hits         17191    17207      +16     
- Misses        5933     5939       +6     
- Partials      1171     1176       +5     
Flag Coverage Δ
all-tests 70.75% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
planner/filter/normalize.go 100.00% <100.00%> (+6.86%) ⬆️

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db1f41c...21498d1. Read the comment docs.

@nasdf nasdf requested a review from a team September 27, 2023 17:46
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much for fixing this

@nasdf nasdf merged commit 2c862ac into sourcenetwork:develop Sep 27, 2023
16 checks passed
@nasdf nasdf deleted the nasdf/fix/flaky-filter-test branch September 27, 2023 18:18
@nasdf nasdf mentioned this pull request Sep 27, 2023
11 tasks
nasdf added a commit that referenced this pull request Oct 2, 2023
## Relevant issue(s)

Closes #1472
Closes #1507
Closes #1860 

## Description

This is a follow up to #1776

This PR adds a CLI implementation that implements the client.DB
interface and runs through the existing integration test suite.

- [x] Merge existing server config code
- [x] Refactor CLI to use new HTTP client
- [x] Remove `net/api` package
- [x] Remove `api/http` package
- [x] Lens tests are timing out in CI: fixed #1862
- [x] Code coverage is incorrectly reporting: fixed #1861
- [x] Flaky test causing failures: fixed #1912

Renamed Commands:
- `peerid` to `client peer info`
- `client p2pcollection` to `client p2p collection`
- `client replicator` to `client p2p replicator`
- `client schema list` to `client collection describe`

Removed Commands:
- `block get`
- `ping`
- `rpc`

Added Commands:
- `client collection create`
- `client collection delete`
- `client collection get`
- `client collection keys`
- `client collection update`
- `client tx create`
- `client tx discard`
- `client tx commit`
- `client schema migration up`
- `client schema migration down`
- `client schema migration reload`

**Notes for reviewers**:
- `.github` changes are merged from #1871
- `Makefile` most of these changes are also from #1871
- `docs/cli` ignore these changes, it will be updated next release
- sorry for all of the merge commits, I am working on learning rebase
flow

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

`make test`

Specify the platform(s) on which this was tested:
- MacOS
shahzadlone pushed a commit that referenced this pull request Oct 2, 2023
## Relevant issue(s)

Resolves #1879 

## Description

This PR attempts to fix a flaky filter test. The normalization logic has
been refactored to a recursive / functional approach that (hopefully)
makes it a little easier to understand.

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Ran normalize tests in a loop for 200k+ iterations.

Specify the platform(s) on which this was tested:
- MacOS
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
## Relevant issue(s)

Resolves sourcenetwork#1879 

## Description

This PR attempts to fix a flaky filter test. The normalization logic has
been refactored to a recursive / functional approach that (hopefully)
makes it a little easier to understand.

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Ran normalize tests in a loop for 200k+ iterations.

Specify the platform(s) on which this was tested:
- MacOS
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
## Relevant issue(s)

Closes sourcenetwork#1472
Closes sourcenetwork#1507
Closes sourcenetwork#1860 

## Description

This is a follow up to sourcenetwork#1776

This PR adds a CLI implementation that implements the client.DB
interface and runs through the existing integration test suite.

- [x] Merge existing server config code
- [x] Refactor CLI to use new HTTP client
- [x] Remove `net/api` package
- [x] Remove `api/http` package
- [x] Lens tests are timing out in CI: fixed sourcenetwork#1862
- [x] Code coverage is incorrectly reporting: fixed sourcenetwork#1861
- [x] Flaky test causing failures: fixed sourcenetwork#1912

Renamed Commands:
- `peerid` to `client peer info`
- `client p2pcollection` to `client p2p collection`
- `client replicator` to `client p2p replicator`
- `client schema list` to `client collection describe`

Removed Commands:
- `block get`
- `ping`
- `rpc`

Added Commands:
- `client collection create`
- `client collection delete`
- `client collection get`
- `client collection keys`
- `client collection update`
- `client tx create`
- `client tx discard`
- `client tx commit`
- `client schema migration up`
- `client schema migration down`
- `client schema migration reload`

**Notes for reviewers**:
- `.github` changes are merged from sourcenetwork#1871
- `Makefile` most of these changes are also from sourcenetwork#1871
- `docs/cli` ignore these changes, it will be updated next release
- sorry for all of the merge commits, I am working on learning rebase
flow

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

`make test`

Specify the platform(s) on which this was tested:
- MacOS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/planner Related to the planner system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestNormalizeConditions is flaky
2 participants