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

[BUG] Failing test on master #704

Open
jdmarshall opened this issue Nov 6, 2022 · 5 comments
Open

[BUG] Failing test on master #704

jdmarshall opened this issue Nov 6, 2022 · 5 comments
Labels

Comments

@jdmarshall
Copy link
Contributor

Describe the bug

Configuration file Tests ✓ Objects wrapped with raw should be unmodified ✓ Inner configuration objects wrapped with raw should be unmodified ✓ Supports multiple levels of nesting ✗ Supports keeping promises raw by default » expected 'this is a promise result', got undefined (==) // /Users/jasonmarshall/Projects/cobbler/node-config/node_modules/vows/lib/assert/macros.js:14

I thought this was something on my end, but I pulled master and I'm still getting this, node 8 through 16

Submitting a PR for a failing test for the test suite is ideal.

Expected behavior
Green tests

Screenshots
If applicable, add screenshots to help explain your problem.

Please tell us about your environment:

  • node-config version: 3.3.8
  • node-version: 8-18

Other information

@lorenwest
Copy link
Collaborator

Thank you for bringing this to our attention. Not sure where this regression was introduced.

@jdmarshall made their first contribution in #667
@inside made their first contribution in #661
@DigitalGreyHat made their first contribution in #677

Callout to the authors of the last few pull requests - can you verify if your PR introduced this?

@inside
Copy link
Contributor

inside commented Nov 8, 2022

Hi, on master I did:

git bisect start
git bisect bad
git bisect good cfde385c024789ae34d0dd731869f0cc59cd9c4c

Tests are green on the aforementioned commit. I ran npm test for each commit the git bisect command proposed. In the end, git bisect revealed:

405e4082b6b015329d4300911bd6998482953ee7 is the first bad commit
commit 405e4082b6b015329d4300911bd6998482953ee7
Author: Grosan Flaviu Gheorghe <fgheorghe@grosan.co.uk>
Date:   Sat Dec 1 11:10:18 2018 +0100

    Fixed property mutation. Throw an exception on such an attempt. Updated tests to capture this exception.

 lib/config.js         | 23 ++++++++++++++++-------
 test/2-config-test.js | 10 +++++++---
 2 files changed, 23 insertions(+), 10 deletions(-)

I haven't inspected what's in this commit. @fgheorghe can you tell if this is the needle we are looking for?

@jdmarshall
Copy link
Contributor Author

That's quite a while ago. Has that test really been failing that long or did something go weird with a merge?

@markstos
Copy link
Collaborator

I'm not sure.

@markstos
Copy link
Collaborator

markstos commented May 1, 2023

I did my own bisect and got the same result. I also tried Node 8 and it failed on that test too. I also checked to see if maybe the test runner had changed over time, so the problem was actually a missing module in node_modules, but I checked that too.

But reviewing the commit, while it has an author date from 2018, it was committed in February of 2022, just a few months before this was flagged in November 2022, so that makes more sense.

The first release with failing test would have been 3.3.8, and the current version is 3.3.9, so that makes more sense.

405e408

@fgheorghe It looks like your commit caused a test failure. Could you take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants