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

[Feature Anywhere] More bug fixes #4269

Merged

Conversation

ohltyler
Copy link
Member

@ohltyler ohltyler commented Jun 9, 2023

Description

Fixes the following:

  • The color of the event and rules when there are events overlaid on a visualization
  • Fixes unit tests due to changed logic in eligibility functions. Adds regression tests for the added logic (empty vislayers makes the actions ineligible)

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #4269 (6c5888c) into feature/feature-anywhere (bcc8f94) will increase coverage by 39.95%.
The diff coverage is 100.00%.

@@                      Coverage Diff                      @@
##           feature/feature-anywhere    #4269       +/-   ##
=============================================================
+ Coverage                     26.39%   66.35%   +39.95%     
=============================================================
  Files                           150     3271     +3121     
  Lines                          3554    62943    +59389     
  Branches                        407     9741     +9334     
=============================================================
+ Hits                            938    41763    +40825     
- Misses                         2592    18838    +16246     
- Partials                         24     2342     +2318     
Flag Coverage Δ
Linux 66.29% <100.00%> (?)
Windows 66.29% <100.00%> (+39.90%) ⬆️

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

Impacted Files Coverage Δ
src/plugins/vis_augmenter/public/test_constants.ts 100.00% <ø> (ø)
src/plugins/vis_augmenter/public/constants.ts 100.00% <100.00%> (ø)
src/plugins/vis_augmenter/public/mocks.ts 100.00% <100.00%> (ø)
src/plugins/vis_augmenter/public/utils/utils.ts 90.00% <100.00%> (ø)

... and 3173 files with indirect coverage changes

@joshuarrrr
Copy link
Member

Unit tests are running again, but with one failure:

Summary of all failing tests
 FAIL  src/plugins/vis_augmenter/public/utils/utils.test.ts (11.739 s)
  ● utils › isEligibleForVisLayers › vis is ineligible with no seriesParams

    expect(received).toEqual(expected) // deep equality

    Expected: false
    Received: undefined

      254 |         },
      255 |       } as unknown) as Vis;
    > 256 |       expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
          |                                                  ^
      257 |     });
      258 |     it('vis is ineligible with valid type and disabled setting', async () => {
      259 |       uiSettingsMock.get.mockImplementation((key: string) => {

      at Object.<anonymous> (src/plugins/vis_augmenter/public/utils/utils.test.ts:256:50)


Test Suites: 1 failed, 1 skipped, 1544 passed, 1545 of 1546 total
Tests:       1 failed, 34 skipped, 9 todo, 12155 passed, 12199 total
Snapshots:   2922 passed, 2922 total
Time:        4462.054 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

@ohltyler
Copy link
Member Author

ohltyler commented Jun 13, 2023

Unit tests are running again, but with one failure:

Summary of all failing tests
 FAIL  src/plugins/vis_augmenter/public/utils/utils.test.ts (11.739 s)
  ● utils › isEligibleForVisLayers › vis is ineligible with no seriesParams

    expect(received).toEqual(expected) // deep equality

    Expected: false
    Received: undefined

      254 |         },
      255 |       } as unknown) as Vis;
    > 256 |       expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
          |                                                  ^
      257 |     });
      258 |     it('vis is ineligible with valid type and disabled setting', async () => {
      259 |       uiSettingsMock.get.mockImplementation((key: string) => {

      at Object.<anonymous> (src/plugins/vis_augmenter/public/utils/utils.test.ts:256:50)


Test Suites: 1 failed, 1 skipped, 1544 passed, 1545 of 1546 total
Tests:       1 failed, 34 skipped, 9 todo, 12155 passed, 12199 total
Snapshots:   2922 passed, 2922 total
Time:        4462.054 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

Let me pull a rebase. Seems this is due to a previous merged change so I will fix here as well.

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@lezzago lezzago merged commit 1009efe into opensearch-project:feature/feature-anywhere Jun 15, 2023
@ohltyler ohltyler deleted the bug-fix-3 branch June 15, 2023 18:45
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